Improve ct_master #8946
Open
Improve ct_master #8946
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The ct_run:run_test function already takes care of the node's logs. The ct_master_logs module takes care of ct_master itself.
Needed to file:set_cwd like in normal CT.
Before this commit the CT docs were lying as ct_master only handled a small number of testspec instructions. This commit fixes that.
It makes more sense to sort by node name, than to have the results in the order they finished.
Breaking change: instead of returning just `ok` to indicate that the spec file was handled, we return an OK tuple with the results of the tests (number of successful, failed, user and auto skipped tests). This allows the caller to know whether any test error occurred.
At the end of a ct_master run. This uses the builtin CT Master event handler to gather the results.
CT Test Results 2 files 58 suites 1h 24m 23s ⏱️ For more details on these failures, see this check. Results for commit fd46027. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
|
The test failure is because of the breaking change mentioned earlier. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At RabbitMQ we have started using
ct_masterto greatly speed up our test runs in CI, moving away from an approach that was caching test results and guessing what tests we had to run again, to an approach that runs all tests but with greater concurrency. (We changed approaches due to circumstances beyond our control, not based on technical merit, but that's a story for another time.)Greater concurrency here means running multiple test suites at the same time in a single machine (the same that
ct_masterruns on).With
ct_masterwe quickly were able to run all test suites in therabbitapplication (https://github.com/rabbitmq/rabbitmq-server/tree/main/deps/rabbit/test), and there are a lot, in under 30 minutes on our development machines.We pushed forward and applied the same principles to CI for our two biggest applications and were able to cut down the run time to around 13 minutes using 4 workers for
rabbit, and 1 worker forrabbitmq_mqtt, both usingct_master. Our other applications are smaller and have not needed this treatment applied to them.I am also interested in making this parallel execution a feature of Erlang.mk at a later time, when the needed functionality is available in OTP directly.
While it is functional,
ct_masteris in a fairly bad state, and this PR aims to improve that. It includes a fix for #8911 as well as additional functionality. Most of the changes are not controversial, although the last two commits may be:ct_masteroutside us (and we are using a forked module).I also noted that the ct_master_status module appears to be completely unused, happy to add a commit to delete it.
The equivalent RabbitMQ PR is at rabbitmq/rabbitmq-server#12502 and one of the comments there links to a few test runs that use
ct_masterwith all changes included in our PR here.Note that I have not run (or added to!) OTP's CT tests at this point, hoping to get some feedback on the more controversial points, and whether
mastershould be the target or ifmaintwould be OK.cc @lhoguin