Change pants_integration_test.py to use Pytest-style functions
#10675
Conversation
# 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]
# 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
requested review from
stuhood,
benjyw,
cosmicexplorer and
gshuflin
| return temporary_dir(root_dir=root, cleanup=cleanup, suffix=".pants.d") | ||
|
|
||
| @contextmanager | ||
| def overwrite_file_content( |
There was a problem hiding this comment.
This was moved to contextutil.py.
| @ensure_daemon | ||
| def test_goal_validation(): | ||
| result = run_pants(["blah", "::"]) | ||
| result.assert_failure() | ||
| assert "Unknown goals: blah" in result.stdout |
There was a problem hiding this comment.
Same as before. Only moved out of the test class, which as being skipped.
| def test_unimplemented_goals_noop() -> None: | ||
| # Running on a `files` target should usually fail, but it should no-op if no `run` | ||
| # implementations are activated. | ||
| with setup_tmpdir({"bad.txt": "", "BUILD": "files(sources=['f.txt')"}) as tmpdir: | ||
| run_pants(["run", tmpdir]).assert_success() | ||
| run_pants(["--backend-packages=['pants.backend.python']", "run", tmpdir]).assert_failure() |
There was a problem hiding this comment.
Moved out of the test class, along with having a fix. The old test only passed because of the v1 target definitions for Python target types.
| @ensure_daemon | ||
| def test_list(self): |
There was a problem hiding this comment.
Deleted. We already have several tests for lint, both unit tests and integration tests.
# 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]
|
|
||
|
|
||
| @contextmanager | ||
| def setup_tmpdir(files: Mapping[str, str]) -> Iterator[str]: |
There was a problem hiding this comment.
This feels like it should be a fixture.
For example, the fixture could take a marker that provides the files, but if that marker isn't provided it could default to taking the value of SOURCES in the requesting module, or something like that.
See, e.g., https://docs.pytest.org/en/stable/fixture.html#using-markers-to-pass-data-to-fixtures and https://docs.pytest.org/en/stable/fixture.html#fixtures-can-introspect-the-requesting-test-context.
|
|
||
|
|
||
| @contextmanager | ||
| def temporary_workdir(cleanup: bool = True) -> Iterator[str]: |
There was a problem hiding this comment.
Ditto. There is already a tmpdir fixture, so tmp_workdir is more idiomatic.
|
I agree fixtures would be neat. I'm not sure how to handle though that these need to be called as normal functions too, such as with temporary_workdir() as workdir:
return run_pants_with_workdir(
command,
workdir=workdir,
hermetic=hermetic,
use_pantsd=use_pantsd,
config=config,
stdin_data=stdin_data,
extra_env=extra_env,
**kwargs,
)How do you feel about landing this as is and saving fixtures as an enhancement? Or is the solution to make those private functions, and have the fixtures + the non-fixture code call those private functions? |
|
We can fixturize later, it's not that critical. |
|
But maybe add TODOs (linked to an issue). |
We've slowly been chipping away at using Pytest-style tests, meaning using
assertrather thanself.assertXand using top-level functions rather than test classes.Now, integration tests can be written in Pytest style. We deprecate the old class-based style.
To land this change, we drop the ability to set the PANTS_PROFILE in an integration test. It appears that this was not used; the workaround is to run the test like you would on the command line.