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

test/CMakeLists.txt: updated the test fixture costs #5565

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

firewave
Copy link
Collaborator

@firewave firewave commented Oct 17, 2023

No description provided.

@firewave firewave changed the title test/CMakeLists.txt: update the default test fixtures costs test/CMakeLists.txt: updated the default test fixtures costs Oct 17, 2023
@firewave firewave marked this pull request as draft October 17, 2023 15:27
@firewave
Copy link
Collaborator Author

firewave commented Oct 17, 2023

Actually to properly calculate the costs locally you should not have any values set. need to re-generate the order.

@firewave
Copy link
Collaborator Author

The stored costs can be found under <cmake-build-folder>/Testing/Temporary/CTestCostData.txt

@firewave firewave marked this pull request as ready for review October 17, 2023 16:32
@firewave firewave changed the title test/CMakeLists.txt: updated the default test fixtures costs test/CMakeLists.txt: updated the test fixture costs Oct 17, 2023
@firewave
Copy link
Collaborator Author

This already takes the improvements from #5566 into consideration.

@danmar danmar merged commit 27bd972 into danmar:main Oct 21, 2023
74 checks passed
@firewave firewave deleted the test-cost branch October 21, 2023 09:05
# To collect data to update this list remove "<cmake-build-folder>/Testing/Temporary/CTestCostData.txt",
# disable the fixture_cost() statements below and run a Debug build with "ctest -j1" several times.
# Afterwards run it with "ctest -j11" and immediately cancel the run and update the list accordingly to the
# first eleven tests chosen.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why only the first 11? We should be using the top slowest tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it is kind of arbitrarily - it was because my machine has 12 threads. I also didn't want to have a list which is too long. But that is also about the amount of tests which run longer than the rest and delivers the desired result with -j2 (what is what we have in the CI).

This should only be applied when using the CI since locally CTest will track the actual times it takes and re-order them. Unfortunately I have seen those costs getting messed up pretty easily so they became useless. That's why I didn't make it optional yet. Still need to file bug reports about it.

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.

3 participants