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 fix, add NPE protection in TestCloudEventCallbackProperty #2704

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

GrantPSpencer
Copy link
Contributor

Issues

#2700 [Failed CI Test] testUserDefinedCallback

Description

afterTest cleanup method assumes that callbackProperty has already been instantiated and is not null. Tests are run sequentially but the order is not guaranteed unless specifically set, so we can't assume the property is not null.

Added null check and also renamed the method for clarity

Tests

  • The following tests are written for this issue:

N/A

  • The following is the result of the "mvn test" command on the appropriate module:

You can reproduce this issue by running the test method on its own using the master branch:

$ mvn test -Dtest=TestCloudEventCallbackProperty#testUserDefinedCallback -pl=helix-core

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.helix.cloud.event.TestCloudEventCallbackProperty
[ERROR] Tests run: 2, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.493 s <<< FAILURE! - in org.apache.helix.cloud.event.TestCloudEventCallbackProperty
[ERROR] testUserDefinedCallback(org.apache.helix.cloud.event.TestCloudEventCallbackProperty)  Time elapsed: 0.01 s  <<< FAILURE!
java.lang.NullPointerException
at org.apache.helix.cloud.event.TestCloudEventCallbackProperty.afterTest(TestCloudEventCallbackProperty.java:64)
at org.apache.helix.cloud.event.TestCloudEventCallbackProperty.testUserDefinedCallback(TestCloudEventCallbackProperty.java:137)

[ERROR] afterTest(org.apache.helix.cloud.event.TestCloudEventCallbackProperty)  Time elapsed: 0.01 s  <<< FAILURE!
java.lang.NullPointerException
at org.apache.helix.cloud.event.TestCloudEventCallbackProperty.afterTest(TestCloudEventCallbackProperty.java:64)

After the change:
$ mvn test -Dtest=TestCloudEventCallbackProperty#testUserDefinedCallback -pl=helix-core

[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.apache.helix.cloud.event.TestCloudEventCallbackProperty
Using handler: org.apache.helix.cloud.event.CloudEventHandler
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.505 s - in org.apache.helix.cloud.event.TestCloudEventCallbackProperty
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO]
[INFO] --- jacoco:0.8.6:report (generate-code-coverage-report) @ helix-core ---
[INFO] Loading execution data file /Users/gspencer/Desktop/git-repos/helix/helix-core/target/jacoco.exec
[INFO] Analyzed bundle 'Apache Helix :: Core' with 802 classes
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS

Copy link
Contributor

@himanshukandwal himanshukandwal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix, @GrantPSpencer

@GrantPSpencer
Copy link
Contributor Author

Failed due to issue #2693 - flaky testCacheDataUpdates, which should be fixed by open PR
#2705

@GrantPSpencer
Copy link
Contributor Author

Pull request approved by @himanshukandwal
Commit message: Fix TestCloudEventCallbackProperty flaky test

@xyuanlu xyuanlu merged commit bf80a54 into apache:master Nov 27, 2023
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants