(PUP-6398) Adjust specs / code organization to support parallel specs on Windows #5114
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
Executing parallel specs on Windows is not reliable in part, because the module tool shares file system state across specs. Previously, executing `rake parallel:spec` on Windows would fail, even when passing on other platforms. This commit is one of several that will allow specs to be executed in parallel by: * Fixing colliding directory read/writes in installer_spec and upgrader_spec by stubbing Puppet.settings[:module_working_dir] to a new directory before each test in installer_spec and in upgrader_spec. This issue only appears when running these tests concurrently unpacker_spec was changed to follow a similar pattern due to discussion about Puppet settings maybe becoming immutable after first load in the future.
Executing parallel specs on Windows is not reliable in part, because of how specs define Puppet::Util::Plist on platforms that don't have cfpropertylist support. Previously, executing `rake parallel:spec` on Windows would fail, even when passing on other platforms. This commit is one of several that will allow specs to be executed in parallel by: * Moving the cfpropertylist code guards from tests to plist.rb, so that any code may require 'util/plist' even if the current platform does not have cfpropertylist support (detectable via Puppet.features.cfpropertylist?). This has the benefit of removing definitions of Puppet::Util::Plist from specs designed to run on all platforms. Previously pkgdmg, launchd tests required Puppet::Util::Plist to be explicitly declared inside the specs to be able to execute on platforms without cfpropertylist support - which could cause a race when run in parallel. Note that prior to this, spec\unit\provider\service\launchd_spec.rb and spec\unit\provider\package\appdmg_spec.rb could not be run in isolation on Windows
On Windows, $CHILD_STATUS is nil while the tests in
spec/unit/provider/service/*.rb. and pkg_spec are running.
This results in a "Can't modify frozen object"
runtime error. However, the tests are able to pass
when run serially with the tests in capability_spec, for
example. The reason being, some tests in capability_spec
make a compile_to_catalog() call which, as a side
effect, generates a pid. $CHILD_STATUS is able to
latch on to this pid, resolving the issue.
The bug shows up when the tests are run in isolation or in
parallel (on occasion, when the tests are grouped in
such a way and executed in such an order that
$CHILD_STATUS is not properly set by another test before
these tests run).
This commit makes the following changes:
* Replaces $CHILD_STATUS.exitstatus calls with stubbed / mocked
Puppet::Util::Execution.exitstatus calls in execution_spec
and pacman_spec
* In the other specs, it generates a pid for $CHILD_STATUS by
spinning up a process.
The previous strategy does not work for these files because
they call into $CHILD_STATUS in lib/provider files, not just
in the specs. And, Puppet::Util::Execution.exitstatus is a
private class method, so that it is not possible to outright
replace these $CHILD_STATUS.exitstatus calls. This may be
resolved with some refactoring in the future.
Also, require 'English' does not set $CHILD_STATUS on
Windows as it does on other platforms. After requiring,
$CHILD_STATUS (and $?) is still nil.
|
CLA signed by all contributors. |
|
When running parallel specs, I'm seeing a few fails, but that may be related to perf issues on my VM? Seen these |
Iristyle
force-pushed
the
pr/5041
branch
2 times, most recently
from
620ded4 to
33465f9
Compare
Executing parallel specs on Windows is not reliable in part, because
the Puppet type system must have been initialized prior to these
settings specs running, which may not be the case when run parallel.
Executing `rake parallel:spec` on Windows would fail, even when
passing on other platforms.
This commit is one of several that will allow specs to be executed in
parallel.
* With Puppet.features.microsoft_windows? stubbed to return false the
first time a settings catalog is built, it will autorequire the
user, file and group type class definitions. When doing this, it
triggers a URI building call through Puppet::Util.path_to_uri to
specify a full path to the actual type implementation code from
puppet/type/{file,user,group}.rb
So when run in isolation, prior to this commit, 3 tests would fail
as a result of this process because it would try to use invalid
non-Windows path to load files.
However, when this test is run after other tests that have loaded
the user / group / file types, then there is no problem as the stub
doesn't impact load behavior.
This fix also addresses being able to run these specs parallel
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 supercedes #5041 with some changes to commits / commit messages