-
Notifications
You must be signed in to change notification settings - Fork 482
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
Integration test for non tokio main #2520
Integration test for non tokio main #2520
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2520 +/- ##
=====================================
Coverage 77.9% 77.9%
=====================================
Files 123 123
Lines 22944 22944
=====================================
Hits 17880 17880
Misses 5064 5064 ☔ View full report in Codecov by Sentry. |
marking draft to fix and isolate the test. |
let mut contents = String::new(); | ||
let mut reader = std::io::BufReader::new(&file); | ||
reader.read_to_string(&mut contents)?; | ||
assert!(contents.contains(expected_content)); |
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.
This doesn't compare the complete JSON, but only the UUID attribute?
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.
yes
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.
I don't think this is the right approach in that case. The test should be capable to validate all the attributes of different types.
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.
I added an alternate approach here - #2521 - running the tests serially with json truncate at start, and letting it validate the complete JSON. We can keep the existing tests as it is, and add new tests to only validate the uuid attribute. This way we have both the scenarios tested?
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.
yes, we need that too. This one is to quickly validate that the shutdown issues are indeed fixed
Fixes #2402
Design discussion issue (if applicable) #
Changes
Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes