Use single-threaded checkout in tests #37
Merged
Conversation
This file contains hidden or 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
This attempts to turn off parallelism within individual runs of writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed by passing `--no-default-features` to turn off the `gix-features-parallel` feature in `gix-worktree-state/Cargo.toml` (which is how `gix-features/parallel` is enabled in those tests). This is to check if the multithreading parallelism within the checkout operations participates in GitoxideLabs#2006. See: GitoxideLabs#2006 (comment)
In the `text-ext4-casefold` jobs.
This moves setting `thread_limit: Some(1)` out of being a step done with `sed` in the `test-ext4-casefold` jobs, and into being an actual change to the source code of the `checkout.rs` test module. This change is intended to be temporary. The goal is the same as before, but to observe the effect outside `text-ext4-casefold`. That is, the goal here is to temporarily see if there is a change in results in the other jobs where failure almost always occurs due to GitoxideLabs#2006, i.e., the `test-fast` jobs on `macos-latest` and `windows-latest`, and the `test-fixtures-windows` job. Note that, as of this change, the `gix-features/parallel` feature is only turned off in the `test-ext4-casefold` jobs. In others, that feature is still turned on, just a parallelism of 1, i.e., a single thread, is used for the checkout. But a parallelism of 1 seems usually to be special-cased to not use facilities related to multithreading. (This `thread_limit` argument change, as well as the immediately preceding change of disabling the `parallel` feature flag in the `test-ext4-casefold` test jobs, are entirely separate from how many writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed duplicated test cases there are, and also entirely separate from the operand to `--test-threads`, which actually controls the number of test *processes* `cargo nextest` creates to run the tests.)
|
Merged into GitoxideLabs#2008 for GitoxideLabs#2006 (comment). |
EliahKagan
added a commit
that referenced
this pull request
May 19, 2025
This change (from #37), which is intended to be temporary, sets `thread_limit: Some(1)` in the `opts_from_probe()` test helper function in the `checkout.rs` test module, so that tests including `writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed` will use single-threaded checkout. This applies no matter how the tests are run. In addition, for the `test-ext4-casefold` CI jobs (but not others), this adds the `--no-default-features` option to the `cargo nextest` command, so that the `gix-features-parallel` feature defined in `gix-worktree-state/Cargo.toml`, which when enabled causes `gix-features/parallel` to be enabled for the tests, is not enabled. The purpose of these changes it to examine if, when checkout operations are themselves performed without multithreading, there is any change to GitoxideLabs#2006. This does not make the failures go away. They still occur on all systems on which they had occurred before. It is unclear if it makes any difference to the failures. It seems like they may be slightly less likely to occur on GNU/Linux, but experiments so far are insufficient to confirm this. Furthermore, even if there is an effect, it may be that the effect is more subtle (such as possibly shifting the number of parallel tests where failures peak, from 2 to some higher number). The changes here should not be confused with the duplication of the test case, nor with the argument to `--test-threads`, which actually specifies the number of parallel test *processes*, since `cargo nextest` is being used. This squashes a few commits trying some minor variations. The original messages are shown below. For full changes and CI results from each commit, see: #37 --- * Try to avoid parallel checkout in `test-ext4-casefold` This attempts to turn off parallelism within individual runs of writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed by passing `--no-default-features` to turn off the `gix-features-parallel` feature in `gix-worktree-state/Cargo.toml` (which is how `gix-features/parallel` is enabled in those tests). This is to check if the multithreading parallelism within the checkout operations participates in GitoxideLabs#2006. See: GitoxideLabs#2006 (comment) * Patch the `thread_limit` argument to `Some(1)` too In the `text-ext4-casefold` jobs. * Modify the test code to use single-threaded checkout This moves setting `thread_limit: Some(1)` out of being a step done with `sed` in the `test-ext4-casefold` jobs, and into being an actual change to the source code of the `checkout.rs` test module. This change is intended to be temporary. The goal is the same as before, but to observe the effect outside `text-ext4-casefold`. That is, the goal here is to temporarily see if there is a change in results in the other jobs where failure almost always occurs due to GitoxideLabs#2006, i.e., the `test-fast` jobs on `macos-latest` and `windows-latest`, and the `test-fixtures-windows` job. Note that, as of this change, the `gix-features/parallel` feature is only turned off in the `test-ext4-casefold` jobs. In others, that feature is still turned on, just a parallelism of 1, i.e., a single thread, is used for the checkout. But a parallelism of 1 seems usually to be special-cased to not use facilities related to multithreading. (This `thread_limit` argument change, as well as the immediately preceding change of disabling the `parallel` feature flag in the `test-ext4-casefold` test jobs, are entirely separate from how many writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed duplicated test cases there are, and also entirely separate from the operand to `--test-threads`, which actually controls the number of test *processes* `cargo nextest` creates to run the tests.)
EliahKagan
added a commit
that referenced
this pull request
May 19, 2025
This change (from #37), which is intended to be temporary, sets `thread_limit: Some(1)` in the `opts_from_probe()` test helper function in the `checkout.rs` test module, so that tests including `writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed` will use single-threaded checkout. This applies no matter how the tests are run. In addition, for the `test-ext4-casefold` CI jobs (but not others), this adds the `--no-default-features` option to the `cargo nextest` command, so that the `gix-features-parallel` feature defined in `gix-worktree-state/Cargo.toml`, which when enabled causes `gix-features/parallel` to be enabled for the tests, is not enabled. The purpose of these changes it to examine if, when checkout operations are themselves performed without multithreading, there is any change to GitoxideLabs#2006. This does not make the failures go away. They still occur on all systems on which they had occurred before. It is unclear if it makes any difference to the failures. It seems like they may be slightly less likely to occur on GNU/Linux, but experiments so far are insufficient to confirm this. Furthermore, even if there is an effect, it may be that the effect is more subtle (such as possibly shifting the number of parallel tests where failures peak, from 2 to some higher number). The changes here should not be confused with the duplication of the test case, nor with the argument to `--test-threads`, which actually specifies the number of parallel test *processes*, since `cargo nextest` is being used. This squashes a few commits trying some minor variations. The original messages are shown below. For full changes and CI results from each commit, see: #37 --- * Try to avoid parallel checkout in `test-ext4-casefold` This attempts to turn off parallelism within individual runs of writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed by passing `--no-default-features` to turn off the `gix-features-parallel` feature in `gix-worktree-state/Cargo.toml` (which is how `gix-features/parallel` is enabled in those tests). This is to check if the multithreading parallelism within the checkout operations participates in GitoxideLabs#2006. See: GitoxideLabs#2006 (comment) * Patch the `thread_limit` argument to `Some(1)` too In the `text-ext4-casefold` jobs. * Modify the test code to use single-threaded checkout This moves setting `thread_limit: Some(1)` out of being a step done with `sed` in the `test-ext4-casefold` jobs, and into being an actual change to the source code of the `checkout.rs` test module. This change is intended to be temporary. The goal is the same as before, but to observe the effect outside `text-ext4-casefold`. That is, the goal here is to temporarily see if there is a change in results in the other jobs where failure almost always occurs due to GitoxideLabs#2006, i.e., the `test-fast` jobs on `macos-latest` and `windows-latest`, and the `test-fixtures-windows` job. Note that, as of this change, the `gix-features/parallel` feature is only turned off in the `test-ext4-casefold` jobs. In others, that feature is still turned on, just a parallelism of 1, i.e., a single thread, is used for the checkout. But a parallelism of 1 seems usually to be special-cased to not use facilities related to multithreading. (This `thread_limit` argument change, as well as the immediately preceding change of disabling the `parallel` feature flag in the `test-ext4-casefold` test jobs, are entirely separate from how many writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed duplicated test cases there are, and also entirely separate from the operand to `--test-threads`, which actually controls the number of test *processes* `cargo nextest` creates to run the tests.)
EliahKagan
added a commit
that referenced
this pull request
May 22, 2025
This change (from #37), which is intended to be temporary, sets `thread_limit: Some(1)` in the `opts_from_probe()` test helper function in the `checkout.rs` test module, so that tests including `writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed` will use single-threaded checkout. This applies no matter how the tests are run. In addition, for the `test-ext4-casefold` CI jobs (but not others), this adds the `--no-default-features` option to the `cargo nextest` command, so that the `gix-features-parallel` feature defined in `gix-worktree-state/Cargo.toml`, which when enabled causes `gix-features/parallel` to be enabled for the tests, is not enabled. The purpose of these changes it to examine if, when checkout operations are themselves performed without multithreading, there is any change to GitoxideLabs#2006. This does not make the failures go away. They still occur on all systems on which they had occurred before. It is unclear if it makes any difference to the failures. It seems like they may be slightly less likely to occur on GNU/Linux, but experiments so far are insufficient to confirm this. Furthermore, even if there is an effect, it may be that the effect is more subtle (such as possibly shifting the number of parallel tests where failures peak, from 2 to some higher number). The changes here should not be confused with the duplication of the test case, nor with the argument to `--test-threads`, which actually specifies the number of parallel test *processes*, since `cargo nextest` is being used. This squashes a few commits trying some minor variations. The original messages are shown below. For full changes and CI results from each commit, see: #37 --- * Try to avoid parallel checkout in `test-ext4-casefold` This attempts to turn off parallelism within individual runs of writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed by passing `--no-default-features` to turn off the `gix-features-parallel` feature in `gix-worktree-state/Cargo.toml` (which is how `gix-features/parallel` is enabled in those tests). This is to check if the multithreading parallelism within the checkout operations participates in GitoxideLabs#2006. See: GitoxideLabs#2006 (comment) * Patch the `thread_limit` argument to `Some(1)` too In the `text-ext4-casefold` jobs. * Modify the test code to use single-threaded checkout This moves setting `thread_limit: Some(1)` out of being a step done with `sed` in the `test-ext4-casefold` jobs, and into being an actual change to the source code of the `checkout.rs` test module. This change is intended to be temporary. The goal is the same as before, but to observe the effect outside `text-ext4-casefold`. That is, the goal here is to temporarily see if there is a change in results in the other jobs where failure almost always occurs due to GitoxideLabs#2006, i.e., the `test-fast` jobs on `macos-latest` and `windows-latest`, and the `test-fixtures-windows` job. Note that, as of this change, the `gix-features/parallel` feature is only turned off in the `test-ext4-casefold` jobs. In others, that feature is still turned on, just a parallelism of 1, i.e., a single thread, is used for the checkout. But a parallelism of 1 seems usually to be special-cased to not use facilities related to multithreading. (This `thread_limit` argument change, as well as the immediately preceding change of disabling the `parallel` feature flag in the `test-ext4-casefold` test jobs, are entirely separate from how many writes_through_symlinks_are_prevented_even_if_overwriting_is_allowed duplicated test cases there are, and also entirely separate from the operand to `--test-threads`, which actually controls the number of test *processes* `cargo nextest` creates to run the tests.)
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.
This change, which is intended to be temporary, sets
thread_limit: Some(1)in theopts_from_probe()test helper function in thecheckout.rstest module, so that tests includingwrites_through_symlinks_are_prevented_even_if_overwriting_is_allowedwill use single-threaded checkout. This applies no matter how the tests are run.In addition, for the
test-ext4-casefoldCI jobs (but not others), this adds the--no-default-featuresoption to thecargo nextestcommand, so that thegix-features-parallelfeature defined ingix-worktree-state/Cargo.toml, which when enabled causesgix-features/parallelto be enabled for the tests, is not enabled.The purpose of these changes it to examine if, when checkout operations are themselves performed without multithreading, there is any change to GitoxideLabs#2006.
This does not make the failures go away. They still occur on all systems on which they had occurred before. It is unclear if it makes any difference to the failures. It seems like they may be slightly less likely to occur on GNU/Linux, but experiments so far are insufficient to confirm this. Furthermore, even if there is an effect, it may be that the effect is more subtle (such as possibly shifting the number of parallel tests where failures peak, from 2 to some higher number).
The changes here should not be confused with the duplication of the test case, nor with the argument to
--test-threads, which actually specifies the number of parallel test processes, sincecargo nextestis being used.This squashes a few commits trying some minor variations. The original messages are shown below. For full changes and CI results from each commit, see:
#37