Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modernize scheduler_test.py and process_test.py #11117

Merged
merged 3 commits into from
Nov 10, 2020

Conversation

Eric-Arellano
Copy link
Contributor

This rewrites them to use RuleRunner/Pytest, and also removes some complexity in process_test.py.

[ci skip-rust]
[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]
Comment on lines -43 to -44
@dataclass(frozen=True)
class Concatted:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The below code was all deleted. It's no longer necessary now that we dogfood the engine so much. Too much complexity to be worth keeping.


def test_multiple_file_creation(self):
digest = self.request(
def test_argv_executable(rule_runner: RuleRunner) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test_executable and test_not_executable tests were merged into this.

QueryRule(FallibleProcessResult, (Process,)),
)

def test_input_file_creation(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was merged with test_multiple_file_creation and test_file_in_directory_creation into a single test lower in the file.

Comment on lines -221 to -223
def test_env(self):
req = Process(argv=("foo",), description="Some process", env={"VAR": "VAL"})
assert dict(req.env) == {"VAR": "VAL"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This original test wasn't really testing much..rewritten to be more comprehensive.

assert dict(req.env) == {"VAR": "VAL"}
def test_env(rule_runner: RuleRunner) -> None:
with environment_as(VAR1="VAL"):
process = Process(argv=("/usr/bin/env",), description="", env={"VAR2": "VAL"})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that this is the correct path on both Linux and macOS.

Comment on lines -270 to +100
def test_fallible_failing_command_returns_exited_result(self):
request = Process(argv=("/bin/bash", "-c", "exit 1"), description="one-cat")
def test_failing_process(rule_runner: RuleRunner) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I merged this test with test_non_fallible_failing_command_raises.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling 501d876 on Eric-Arellano:port-tests into b7390ef on pantsbuild:master.

@Eric-Arellano Eric-Arellano merged commit 62cd7bc into pantsbuild:master Nov 10, 2020
@Eric-Arellano Eric-Arellano deleted the port-tests branch November 10, 2020 00:28
@Eric-Arellano Eric-Arellano mentioned this pull request Nov 10, 2020
Eric-Arellano added a commit that referenced this pull request Nov 10, 2020
This leaves off these internal only changes:

* Remove the examples/ directory. (#11100)
  `PR #11100 <https://github.com/pantsbuild/pants/pull/11100>`_

* Upgrade toml from 0.10.1 to 0.10.2 (#11097)
  `PR #11097 <https://github.com/pantsbuild/pants/pull/11097>`_

* Modernize `scheduler_test.py` and `process_test.py` (#11117)

[ci skip-rust]
[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants