-
Notifications
You must be signed in to change notification settings - Fork 221
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
Migrate parameterized DroidBench tests from JUnit 4 to JUnit 5 #1298
Conversation
Test Results 451 files ± 0 451 suites ±0 2h 48m 2s ⏱️ - 1m 40s Results for commit ab2f5ee. ± Comparison against base commit 70eb6e9. This pull request removes 232 and adds 116 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
3cd2490
to
fd91301
Compare
Do we expect the discrepancy between removed and added tests in the test results report? |
We do not. I'm still investigating, which is why I haven't requested your review yet. I'm glad to see that the new test summary report is serving as an extra check: exactly what I intended. |
fd91301
to
e8f17dc
Compare
IntelliJ IDEA does not know how to migrate parameterized tests, so these test classes were migrated by hand.
e8f17dc
to
ab2f5ee
Compare
In brief: we're not actually losing any tests in this PR. We're simply losing spurious variations in test names. The test summary before this change reports 2,804 test runs and 846 tests. The 846 count is based on distinct test names. A single test name leads to multiple test runs because we test on multiple operating systems and Java versions. The test summary after this change reports 2,804 test runs and 730 tests. So we actually have the same number of test runs across all tested platforms, but these test runs consist of fewer distinct test names. A detailed, manual review of the before/after test names shows that several tests' before names included absolute path names, but are reported using shorter, relative path names after this change. Absolute paths vary by operating system, while the relative names do not. Thus, the before tests had 846 distinct names that reduced to just 730 distinct names after this change. To give one concrete example, tests using the
|
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.
Thanks for the change and for digging in to check it!
You're welcome, @msridhar! I'm really liking these “Test Results” summaries. We’re still getting used to them, but they’re going to be a nice piece of extra protection for us. |
IntelliJ IDEA does not know how to migrate parameterized tests, so these test classes were migrated by hand.