-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add test cases for partially delivered submissions #16055
Add test cases for partially delivered submissions #16055
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
Test Results1 241 tests +2 1 237 ✅ +2 7m 42s ⏱️ -3s Results for commit dee5da9. ± Comparison against base commit 5025bf5. This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
I pulled master and merged it into a local copy of this branch and got the test error you see in the attached report. |
Looks like a connection refused on your local blob container. Investigating further but passing in CI so might be a local issue. |
@@ -153,7 +307,7 @@ class SubmissionFunctionIntegrationTests { | |||
val historyNode = JacksonMapperUtilities.defaultMapper.readTree(history.body.toString()) | |||
assertThat( | |||
historyNode.get("overallStatus").asText() | |||
).isEqualTo(DetailedSubmissionHistory.Status.PARTIALLY_DELIVERED.toString()) | |||
).isEqualTo(DetailedSubmissionHistory.Status.PARTIALLY_DELIVERED.toString()) // TODO: should be RECEIVED |
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.
why the TODO?
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.
Related to the todo in the function header (#16054)
@@ -95,15 +248,16 @@ class SubmissionFunctionIntegrationTests { | |||
val historyNode = JacksonMapperUtilities.defaultMapper.readTree(history.body.toString()) | |||
assertThat( | |||
historyNode.get("overallStatus").asText() | |||
).isEqualTo(DetailedSubmissionHistory.Status.PARTIALLY_DELIVERED.toString()) | |||
).isEqualTo(DetailedSubmissionHistory.Status.PARTIALLY_DELIVERED.toString()) // TODO: should be RECEIVED |
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.
why the TODO?
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.
Related to the todo in the function header (#16054)
@@ -189,6 +208,7 @@ class ReportNodeBuilder { | |||
} | |||
} | |||
lateinit var theAction: TaskAction |
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.
why are some of these lateinit and some of these are nullable vars?
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.
Good point! I'm going to take a 2nd look; on first (2nd) glance they should probably both be nullable since null is a valid state in either case.
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.
lgtm!
Quality Gate passedIssues Measures |
This PR adds tests cases to properly cover a partially delivered submission. It also relabels 2 old tests that are actually testing in-flight submissions. The bug that this PR fixes was actually solved in prior PR(s), but it only added tests for pipeline output and not submission history.
Test Steps:
Changes
ReportNodeBuilder.kt
that got lumped in here for reasonsChecklist
Testing
./prime test
or./gradlew testSmoke
against local Docker ReportStream container?Linked Issues
To Be Done