-
Notifications
You must be signed in to change notification settings - Fork 192
Clean up some test assertions and make sure naming is consistent. #2690
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
Clean up some test assertions and make sure naming is consistent. #2690
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
temporal-sdk/src/test/java/io/temporal/client/functional/UpdateTestTimeoutTest.java
Show resolved
Hide resolved
.../src/test/java/io/temporal/internal/statemachines/CommandsGeneratePlantUMLStateDiagrams.java
Outdated
Show resolved
Hide resolved
maciejdudko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor suggestions but otherwise looks fine. LGTM!
temporal-sdk/src/test/java/io/temporal/internal/worker/ActivityFailedMetricsTests.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/test/java/io/temporal/internal/worker/ActivityFailedMetricsTests.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/test/java/io/temporal/internal/worker/ActivityFailedMetricsTests.java
Outdated
Show resolved
Hide resolved
| assertNotSame( | ||
| "Failure should not be benign", af.getCategory(), ApplicationErrorCategory.BENIGN); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| assertNotSame( | |
| "Failure should not be benign", af.getCategory(), ApplicationErrorCategory.BENIGN); | |
| assertNotEquals( | |
| "Failure should not be benign", af.getCategory(), ApplicationErrorCategory.BENIGN); |
temporal-sdk/src/test/java/io/temporal/internal/testing/ActivityTestingTest.java
Outdated
Show resolved
Hide resolved
2b69659 to
e0281b4
Compare
e0281b4 to
75f60a9
Compare
Clean up some test assertions and make sure naming is consistent. This is mostly just applying IDE recommendation for warnings.
Note
Normalize test assertions and class names, and update PlantUML generator references in tests and generated diagrams.
assertEquals(null/true/false, ...)withassertNull/assertTrue/assertFalse, useassertNotEquals/assertInstanceOf, and fix assertion argument order across many tests.import static org.junit.Assert.*).*Testfor consistency (e.g.,UpdateTestTimeoutTest,WarnUnfinishedHandlersTest,WorkflowDescribeTest, various signal/update tests).CommandsGeneratePlantUMLStateDiagramstoCommandsGeneratePlantUMLStateDiagramsTestin tests and generated.pumlheaders.Written by Cursor Bugbot for commit 75f60a9. This will update automatically on new commits. Configure here.