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

Remove TestDbSuites. Run Query and Route tests directly. #276

Merged
merged 4 commits into from
Jan 9, 2025

Conversation

ggVGc
Copy link
Contributor

@ggVGc ggVGc commented Dec 20, 2024

This removes the TestDbSuites composite suite, which instantiates a single TestDb which is then shared by SparqlRouteTests and SparqlTests and cleaned up when they finish. Instead a new TestDb is now instantiated in each of the tests that use it.
The benefit of this is that we do not have to disable test discovery, which in my experience makes things like filtering on test name break, and makes the reporting less clear since it is always TestDbSuites that runs/fails and we don't see the runtime for each child test. It also seems to me that this increases parallelization, which is a benefit since these two tests seem to be the slowest in the total suite.
What we lose compared to the old version is some extra work for setting up the TestDb twice, however from what I see this is negligible in relation to actually running the tests.

Some rough numbers from my machine for reference:

Old: Clean build + test

[info] Run completed in 23 seconds, 293 milliseconds.
[info] Total number of tests run: 281
[info] Suites: completed 37, aborted 0
[info] Tests: succeeded 281, failed 0, canceled 0, ignored 3, pending 0
[info] All tests passed.
[success] Total time: 64 s (01:04), completed Dec 20, 2024, 6:34:56 PM

Old: Re-run test

[info] Run completed in 23 seconds, 266 milliseconds.
[info] Total number of tests run: 281
[info] Suites: completed 37, aborted 0
[info] Tests: succeeded 281, failed 0, canceled 0, ignored 3, pending 0
[info] All tests passed.
[success] Total time: 26 s, completed Dec 20, 2024, 6:36:57 PM

New: Clean build + test

[info] Run completed in 23 seconds, 265 milliseconds.
[info] Total number of tests run: 281
[info] Suites: completed 36, aborted 0
[info] Tests: succeeded 281, failed 0, canceled 0, ignored 3, pending 0
[info] All tests passed.
[success] Total time: 45 s, completed Dec 20, 2024, 6:38:32 PM

New: Re-run test

[info] Run completed in 25 seconds, 894 milliseconds.
[info] Total number of tests run: 281
[info] Suites: completed 36, aborted 0
[info] Tests: succeeded 281, failed 0, canceled 0, ignored 3, pending 0
[info] All tests passed.
[success] Total time: 28 s, completed Dec 20, 2024, 6:39:34 PM

@ggVGc ggVGc requested a review from mirzov December 20, 2024 17:48
Comment on lines -14 to -16
import se.lu.nateko.cp.meta.test.services.sparql.TestDbFixture
import se.lu.nateko.cp.meta.utils.rdf4j.createDateTimeLiteral
import se.lu.nateko.cp.meta.utils.rdf4j.createLiteral
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to this change, but these imports are unused.


import java.net.URI
import scala.concurrent.Await
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also unused import from before.

@ggVGc ggVGc changed the title [Suggestion] Remove TestDbSuites. Run Query and Route tests directly. Remove TestDbSuites. Run Query and Route tests directly. Jan 9, 2025
@ggVGc ggVGc merged commit ce3f15b into master Jan 9, 2025
1 check passed
@ggVGc ggVGc deleted the valter/split-query-route-tests branch January 9, 2025 12:18
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.

2 participants