Change PantsIntegrationTest to be hermetic by default
#10672
Merged
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
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Eric-Arellano
changed the title
PantsIntegrationTestPantsIntegrationTest
Eric-Arellano
force-pushed
the
refactor-it
branch
from
f598eff
to
7e8a8fe
Compare
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Eric-Arellano
force-pushed
the
refactor-it
branch
from
7e8a8fe
to
007dd16
Compare
Eric-Arellano
commented
Aug 23, 2020
Comment on lines
-173
to
-174
| class InvalidTestEnvironmentError(Exception): | ||
| """Raised when the external environment is not set up properly to run integration tests.""" |
There was a problem hiding this comment.
This was unused, so deleted.
Comment on lines
-216
to
-219
| def setUp(self): | ||
| super().setUp() | ||
| # Some integration tests rely on clean subsystem state (e.g., to set up a DistributionLocator). | ||
| Subsystem.reset() |
There was a problem hiding this comment.
Deleted. No need for this.
Comment on lines
-221
to
-228
| def temporary_workdir(self, cleanup=True): | ||
| # We can hard-code '.pants.d' here because we know that will always be its value | ||
| # in the pantsbuild/pants repo (e.g., that's what we .gitignore in that repo). | ||
| # Grabbing the pants_workdir config would require this pants's config object, | ||
| # which we don't have a reference to here. | ||
| root = os.path.join(get_buildroot(), ".pants.d", "tmp") | ||
| safe_mkdir(root) | ||
| return temporary_dir(root_dir=root, cleanup=cleanup, suffix=".pants.d") |
There was a problem hiding this comment.
Kept, but moved down in the file near the other "temporary" methods.
benjyw
approved these changes
Aug 23, 2020
Eric-Arellano
changed the title
PantsIntegrationTestPantsIntegrationTest to be hermetic by default
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
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.
Several refactors:
hermetica class property rather than classmethod. Default to True.use_pantsd_env_varto a class propertyuse_pantsd. Removes the env var mechanism as we no longer use that. Defaults to pantsd being used.hermetic_env_whitelisttohermetic_env_allowlist, and make it a class property.do_command()in favor ofself.run_pants(). It does the same thing. We want to simplify.file_renamed. We generally are moving in the direction of our integration tests creating their own environment, rather than mutating the build root (e.g. testprojects). This wasn't being used.A followup will make it more convenient to create temporary content, which is a common pattern for newly written integration tests.
[ci skip-rust]
[ci skip-build-wheels]