Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flaky Test: com.facebook.presto.orc.metadata.statistics.TestMapColumnStatisticsBuilder.testAddMapStatisticsNoValues #20807

Closed
ZacBlanco opened this issue Sep 8, 2023 · 8 comments · Fixed by #22303
Assignees

Comments

@ZacBlanco
Copy link
Contributor

CI Run: https://github.com/prestodb/presto/actions/runs/6116066297/job/16600699444?pr=20776

@Akanksha-kedia
Copy link
Contributor

@ZacBlanco [INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.49 s - in com.facebook.presto.orc.metadata.statistics.TestMapColumnStatisticsBuilder
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 11.255 s
[INFO] Finished at: 2023-09-13T19:27:43+05:30

Can you provide more information abt error, steps to recreate the issue.

@ZacBlanco
Copy link
Contributor Author

ZacBlanco commented Sep 13, 2023

This was the log:

Error:  Failures: 
Error:  com.facebook.presto.orc.metadata.statistics.TestMapColumnStatisticsBuilder.testAddMapStatisticsNoValues
[INFO]   Run 1: PASS
Error:    Run 2: TestMapColumnStatisticsBuilder.testAddMapStatisticsNoValues » IllegalState There is no start record for test: com.facebook.presto.orc.metadata.statistics.TestMapColumnStatisticsBuilder::testAddMapStatisticsNoValues
[INFO] 
[INFO] 
Error:  Tests run: 594, Failures: 1, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  15:26 min
[INFO] Finished at: 2023-09-08T00:33:39Z
[INFO] ------------------------------------------------------------------------
Error:  Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M7:test (default-test) on project presto-orc: There are test failures.
Error:  
Error:  Please refer to /home/runner/work/presto/presto/presto-orc/target/surefire-reports for the individual test results.
Error:  Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
Error:  -> [Help 1]
Error:  
Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
Error:  Re-run Maven using the -X switch to enable full debug logging.
Error:  
Error:  For more information about the errors and possible solutions, please read the following articles:
Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
Error: Process completed with exit code 1.

I haven't looked into the cause. I just know that a PR I was working on did not touch anything near this part of the codebase, so it should not have failed

@Akanksha-kedia
Copy link
Contributor

Akanksha-kedia commented Sep 13, 2023 via email

@ZacBlanco
Copy link
Contributor Author

Yes after re-running the test passed

@ZacBlanco
Copy link
Contributor Author

After looking closer, the failure seems to have come from a TestNG extension within Presto, but not the test itself. The error was generated on this line: https://github.com/prestodb/presto/blob/master/presto-testng-services/src/main/java/com/facebook/presto/testng/services/LogTestDurationListener.java#L177

@Akanksha-kedia
Copy link
Contributor

Akanksha-kedia commented Sep 15, 2023 via email

@elharo
Copy link
Contributor

elharo commented Mar 12, 2024

We probably don't need to track the duration of the tests. We could probably just bypass or remove the code completely.

@rschlussel rschlussel self-assigned this Mar 22, 2024
@rschlussel
Copy link
Contributor

the code has been used for debugging test issues (in the past we've had issues where tests would just hang or some tests would run really long), but we probably shouldn't fail the test. Anyway, looks like the issue here is there were two tests in the class with the same name that must have run concurrently, so they shared an entry in this map

private final Map<String, Long> started = new ConcurrentHashMap<>();
.

rschlussel added a commit to rschlussel/presto that referenced this issue Mar 22, 2024
Include method parameters in the test name used in
LogTestDurationListener.  This fixes the listener when there are
multiple tests in the same class with the same name, but different
numbers of parameters.

Also updated the same named tests to have different names because it is
better for tests to use distinct names.

Fixes: prestodb#20807
rschlussel added a commit to rschlussel/presto that referenced this issue Mar 22, 2024
Include method parameters in the test name used in
LogTestDurationListener.  This fixes the listener when there are
multiple tests in the same class with the same name, but different
numbers of parameters.

Also updated the same named tests to have different names because it is
better for tests to use distinct names.

Fixes: prestodb#20807
rschlussel added a commit that referenced this issue Mar 22, 2024
Include method parameters in the test name used in
LogTestDurationListener.  This fixes the listener when there are
multiple tests in the same class with the same name, but different
numbers of parameters.

Also updated the same named tests to have different names because it is
better for tests to use distinct names.

Fixes: #20807
@github-project-automation github-project-automation bot moved this from 🆕 Unprioritized to ✅ Done in Bugs and support requests Mar 22, 2024
@github-project-automation github-project-automation bot moved this from 🆕 Unprioritized to ✅ Done in Build, releases and testing Mar 22, 2024
efrancisworks pushed a commit to efrancisworks/presto that referenced this issue Mar 31, 2024
Include method parameters in the test name used in
LogTestDurationListener.  This fixes the listener when there are
multiple tests in the same class with the same name, but different
numbers of parameters.

Also updated the same named tests to have different names because it is
better for tests to use distinct names.

Fixes: prestodb#20807
gupteaj pushed a commit to gupteaj/presto that referenced this issue Apr 17, 2024
Include method parameters in the test name used in
LogTestDurationListener.  This fixes the listener when there are
multiple tests in the same class with the same name, but different
numbers of parameters.

Also updated the same named tests to have different names because it is
better for tests to use distinct names.

Fixes: prestodb#20807
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants