Skip to content

Commit

Permalink
Add RuleRunner as a Pytest-style replacement to TestBase (pantsbu…
Browse files Browse the repository at this point in the history 
…ild#10699)

## Problem

We want to be able to use Pytest-style tests, rather than unittest style tests. Pytest allows for nice features like parameterization and fixtures, including dozens of pre-built fixtures like `caplog` (capture logging).

For our `TestBase`-style tests, we need a wrapper around a `SchedulerSession` in order to make synchronous calls to the engine. We also need to set up a temporary build root.

For isolation between tests, we must be careful to invalidate every test, such as using a new build root every individual test.

## Solution

Add `RuleRunner`, which is a dataclass around all the config necessary to create a `BuildConfiguration` and `SchedulerSession`. This dataclass exposes the methods `request_product()` and `run_goal_rule()`, along with utilities like `add_to_build_file()`.

Each individual test should create a new instance of a `RuleRunner`. This is important for isolation between tests.

Conventionally, this will be done with a [Pytest fixture](https://docs.pytest.org/en/stable/fixture.html), which allows us to set up common config (e.g. rules and target types) for multiple tests but to still get a distinct `RuleRunner` instance for each test.

```python
@pytest.fixture
def rule_runner() -> RuleRunner:
    return RuleRunner(rules=filedeps.rules(), target_types=[MockTarget, ProtobufLibrary])

def test_no_target(rule_runner: RuleRunner) -> None:
    rule_runner.create_file(...)
    rule_runner.request_product(..)

```

Users can also create the `RuleRunner` inline.

If there are multiple different configurations in a test file, the user may set up multiple different Pytest fixtures.

```python
@pytest.fixture
def target_adaptor_rule_runner() -> RuleRunner:
    return RuleRunner(
        rules=[QueryRule(TargetAdaptor, (Address, OptionsBootstrapper))], target_types=[MockTgt]
    )

...

@pytest.fixture
def address_specs_rule_runner() -> RuleRunner:
    return RuleRunner(
        rules=[QueryRule(AddressesWithOrigins, (AddressSpecs, OptionsBootstrapper))],
        target_types=[MockTgt],
    )
```

## Result

We can always use Pytest style tests now in Pants. We deprecate `TestBase` to be removed in 2.1.0.dev0.

### Performance benchmark

We used to only create one `SchedulerSession` for the entire test class and to invalidate on every individual test. Now, we create a new `SchedulerSession` every single test. This ends up having a negligible performance impact.

Before:

```
multitime -n 10 ./pants --no-pantsd test --force src/python/pants/core/util_rules/filter_empty_sources_test.py
1: ./pants --no-pantsd test --force src/python/pants/core/util_rules/filter_empty_sources_test.py
            Mean        Std.Dev.    Min         Median      Max
real        9.269       0.135       8.976       9.286       9.468
user        9.226       0.143       8.992       9.201       9.498
sys         6.960       0.133       6.768       6.980       7.187
```

After:

```
multitime -n 10 ./pants --no-pantsd test --force src/python/pants/core/util_rules/filter_empty_sources_test.py
            Mean        Std.Dev.    Min         Median      Max
real        9.347       0.125       9.172       9.339       9.666
user        9.332       0.139       9.189       9.299       9.726
sys         7.083       0.123       6.853       7.096       7.359
```

### Risk: breaking test isolation

It is possible for a user to use a single `RuleRunner` for multiple tests, e.g. one instance for the entire test class. This will reduce isolation between tests.

We mitigate this risk by only ever documenting how you should properly use `RuleRunner`, along with adding a warning to https://www.pantsbuild.org/v2.0/docs/rules-api-testing about this risk.

### Unsolved issue: rule registration

It is still clunky to register the rules you need for your test to pass. This should still be solved, but it's left for a followup.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
@Eric-Arellano
Eric-Arellano authored Aug 28, 2020
1 parent 2fc8a42 commit d3eee59
Show file tree
Hide file tree
Showing 19 changed files with 1,521 additions and 1,268 deletions.
335 changes: 177 additions & 158 deletions src/python/pants/backend/project_info/filedeps_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,175 +3,194 @@

from typing import List, Optional, Set

import pytest

from pants.backend.codegen.protobuf.target_types import ProtobufLibrary
from pants.backend.project_info import filedeps
from pants.engine.target import Dependencies, Sources, Target
from pants.testutil.test_base import TestBase
from pants.testutil.rule_runner import RuleRunner


class MockTarget(Target):
alias = "tgt"
core_fields = (Sources, Dependencies)


class FiledepsTest(TestBase):
@classmethod
def rules(cls):
return (*super().rules(), *filedeps.rules())

@classmethod
def target_types(cls):
return [MockTarget, ProtobufLibrary]

def setup_target(
self,
path: str,
*,
sources: Optional[List[str]] = None,
dependencies: Optional[List[str]] = None,
) -> None:
if sources:
self.create_files(path, sources)
self.add_to_build_file(
path,
f"tgt(sources={sources or []}, dependencies={dependencies or []})",
)

def assert_filedeps(
self,
*,
targets: List[str],
expected: Set[str],
transitive: bool = False,
globs: bool = False,
) -> None:
args = []
if globs:
args.append("--filedeps-globs")
if transitive:
args.append("--filedeps-transitive")
result = self.run_goal_rule(filedeps.Filedeps, args=(*args, *targets))
assert result.stdout.splitlines() == sorted(expected)

def test_no_target(self) -> None:
self.assert_filedeps(targets=[], expected=set())

def test_one_target_no_source(self) -> None:
self.setup_target("some/target")
self.assert_filedeps(targets=["some/target"], expected={"some/target/BUILD"})

def test_one_target_one_source(self) -> None:
self.setup_target("some/target", sources=["file.py"])
self.assert_filedeps(
targets=["some/target"], expected={"some/target/BUILD", "some/target/file.py"}
)

def test_one_target_multiple_source(self) -> None:
self.setup_target("some/target", sources=["file1.py", "file2.py"])
self.assert_filedeps(
targets=["some/target"],
expected={"some/target/BUILD", "some/target/file1.py", "some/target/file2.py"},
)

def test_one_target_no_source_one_dep(self) -> None:
self.setup_target("dep/target", sources=["file.py"])
self.setup_target("some/target", dependencies=["dep/target"])
self.assert_filedeps(targets=["some/target"], expected={"some/target/BUILD"})
self.assert_filedeps(
targets=["some/target"],
transitive=True,
expected={"some/target/BUILD", "dep/target/BUILD", "dep/target/file.py"},
)

def test_one_target_one_source_with_dep(self) -> None:
self.setup_target("dep/target", sources=["file.py"])
self.setup_target("some/target", sources=["file.py"], dependencies=["dep/target"])
direct_files = {"some/target/BUILD", "some/target/file.py"}
self.assert_filedeps(targets=["some/target"], expected=direct_files)
self.assert_filedeps(
targets=["some/target"],
transitive=True,
expected={
*direct_files,
"dep/target/BUILD",
"dep/target/file.py",
},
)

def test_multiple_targets_one_source(self) -> None:
self.setup_target("some/target", sources=["file.py"])
self.setup_target("other/target", sources=["file.py"])
self.assert_filedeps(
targets=["some/target", "other/target"],
expected={
"some/target/BUILD",
"some/target/file.py",
"other/target/BUILD",
"other/target/file.py",
},
)

def test_multiple_targets_one_source_with_dep(self) -> None:
self.setup_target("dep1/target", sources=["file.py"])
self.setup_target("dep2/target", sources=["file.py"])
self.setup_target("some/target", sources=["file.py"], dependencies=["dep1/target"])
self.setup_target("other/target", sources=["file.py"], dependencies=["dep2/target"])
direct_files = {
"some/target/BUILD",
"some/target/file.py",
"other/target/BUILD",
"other/target/file.py",
}
self.assert_filedeps(
targets=["some/target", "other/target"],
expected=direct_files,
)
self.assert_filedeps(
targets=["some/target", "other/target"],
transitive=True,
expected={
*direct_files,
"dep1/target/BUILD",
"dep1/target/file.py",
"dep2/target/BUILD",
"dep2/target/file.py",
},
)

def test_multiple_targets_one_source_overlapping(self) -> None:
self.setup_target("dep/target", sources=["file.py"])
self.setup_target("some/target", sources=["file.py"], dependencies=["dep/target"])
self.setup_target("other/target", sources=["file.py"], dependencies=["dep/target"])
direct_files = {
@pytest.fixture
def rule_runner() -> RuleRunner:
return RuleRunner(rules=filedeps.rules(), target_types=[MockTarget, ProtobufLibrary])


def setup_target(
rule_runner: RuleRunner,
path: str,
*,
sources: Optional[List[str]] = None,
dependencies: Optional[List[str]] = None,
) -> None:
if sources:
rule_runner.create_files(path, sources)
rule_runner.add_to_build_file(
path,
f"tgt(sources={sources or []}, dependencies={dependencies or []})",
)


def assert_filedeps(
rule_runner: RuleRunner,
*,
targets: List[str],
expected: Set[str],
transitive: bool = False,
globs: bool = False,
) -> None:
args = []
if globs:
args.append("--filedeps-globs")
if transitive:
args.append("--filedeps-transitive")
result = rule_runner.run_goal_rule(filedeps.Filedeps, args=(*args, *targets))
assert result.stdout.splitlines() == sorted(expected)


def test_no_target(rule_runner: RuleRunner) -> None:
assert_filedeps(rule_runner, targets=[], expected=set())


def test_one_target_no_source(rule_runner: RuleRunner) -> None:
setup_target(rule_runner, "some/target")
assert_filedeps(rule_runner, targets=["some/target"], expected={"some/target/BUILD"})


def test_one_target_one_source(rule_runner: RuleRunner) -> None:
setup_target(rule_runner, "some/target", sources=["file.py"])
assert_filedeps(
rule_runner, targets=["some/target"], expected={"some/target/BUILD", "some/target/file.py"}
)


def test_one_target_multiple_source(rule_runner: RuleRunner) -> None:
setup_target(rule_runner, "some/target", sources=["file1.py", "file2.py"])
assert_filedeps(
rule_runner,
targets=["some/target"],
expected={"some/target/BUILD", "some/target/file1.py", "some/target/file2.py"},
)


def test_one_target_no_source_one_dep(rule_runner: RuleRunner) -> None:
setup_target(rule_runner, "dep/target", sources=["file.py"])
setup_target(rule_runner, "some/target", dependencies=["dep/target"])
assert_filedeps(rule_runner, targets=["some/target"], expected={"some/target/BUILD"})
assert_filedeps(
rule_runner,
targets=["some/target"],
transitive=True,
expected={"some/target/BUILD", "dep/target/BUILD", "dep/target/file.py"},
)


def test_one_target_one_source_with_dep(rule_runner: RuleRunner) -> None:
setup_target(rule_runner, "dep/target", sources=["file.py"])
setup_target(rule_runner, "some/target", sources=["file.py"], dependencies=["dep/target"])
direct_files = {"some/target/BUILD", "some/target/file.py"}
assert_filedeps(rule_runner, targets=["some/target"], expected=direct_files)
assert_filedeps(
rule_runner,
targets=["some/target"],
transitive=True,
expected={
*direct_files,
"dep/target/BUILD",
"dep/target/file.py",
},
)


def test_multiple_targets_one_source(rule_runner: RuleRunner) -> None:
setup_target(rule_runner, "some/target", sources=["file.py"])
setup_target(rule_runner, "other/target", sources=["file.py"])
assert_filedeps(
rule_runner,
targets=["some/target", "other/target"],
expected={
"some/target/BUILD",
"some/target/file.py",
"other/target/BUILD",
"other/target/file.py",
}
self.assert_filedeps(targets=["some/target", "other/target"], expected=direct_files)
self.assert_filedeps(
targets=["some/target", "other/target"],
transitive=True,
expected={*direct_files, "dep/target/BUILD", "dep/target/file.py"},
)

def test_globs(self) -> None:
self.create_files("some/target", ["test1.py", "test2.py"])
self.add_to_build_file("some/target", target="tgt(sources=['test*.py'])")
self.assert_filedeps(
targets=["some/target"],
expected={"some/target/BUILD", "some/target/test*.py"},
globs=True,
)

def test_build_with_file_ext(self) -> None:
self.create_file("some/target/BUILD.ext", contents="tgt()")
self.assert_filedeps(targets=["some/target"], expected={"some/target/BUILD.ext"})

def test_codegen_targets_use_protocol_files(self) -> None:
# That is, don't output generated files.
self.create_file("some/target/f.proto")
self.add_to_build_file("some/target", "protobuf_library()")
self.assert_filedeps(
targets=["some/target"], expected={"some/target/BUILD", "some/target/f.proto"}
)
},
)


def test_multiple_targets_one_source_with_dep(rule_runner: RuleRunner) -> None:
setup_target(rule_runner, "dep1/target", sources=["file.py"])
setup_target(rule_runner, "dep2/target", sources=["file.py"])
setup_target(rule_runner, "some/target", sources=["file.py"], dependencies=["dep1/target"])
setup_target(rule_runner, "other/target", sources=["file.py"], dependencies=["dep2/target"])
direct_files = {
"some/target/BUILD",
"some/target/file.py",
"other/target/BUILD",
"other/target/file.py",
}
assert_filedeps(
rule_runner,
targets=["some/target", "other/target"],
expected=direct_files,
)
assert_filedeps(
rule_runner,
targets=["some/target", "other/target"],
transitive=True,
expected={
*direct_files,
"dep1/target/BUILD",
"dep1/target/file.py",
"dep2/target/BUILD",
"dep2/target/file.py",
},
)


def test_multiple_targets_one_source_overlapping(rule_runner: RuleRunner) -> None:
setup_target(rule_runner, "dep/target", sources=["file.py"])
setup_target(rule_runner, "some/target", sources=["file.py"], dependencies=["dep/target"])
setup_target(rule_runner, "other/target", sources=["file.py"], dependencies=["dep/target"])
direct_files = {
"some/target/BUILD",
"some/target/file.py",
"other/target/BUILD",
"other/target/file.py",
}
assert_filedeps(rule_runner, targets=["some/target", "other/target"], expected=direct_files)
assert_filedeps(
rule_runner,
targets=["some/target", "other/target"],
transitive=True,
expected={*direct_files, "dep/target/BUILD", "dep/target/file.py"},
)


def test_globs(rule_runner: RuleRunner) -> None:
rule_runner.create_files("some/target", ["test1.py", "test2.py"])
rule_runner.add_to_build_file("some/target", target="tgt(sources=['test*.py'])")
assert_filedeps(
rule_runner,
targets=["some/target"],
expected={"some/target/BUILD", "some/target/test*.py"},
globs=True,
)


def test_build_with_file_ext(rule_runner: RuleRunner) -> None:
rule_runner.create_file("some/target/BUILD.ext", contents="tgt()")
assert_filedeps(rule_runner, targets=["some/target"], expected={"some/target/BUILD.ext"})


def test_codegen_targets_use_protocol_files(rule_runner: RuleRunner) -> None:
# That is, don't output generated files.
rule_runner.create_file("some/target/f.proto")
rule_runner.add_to_build_file("some/target", "protobuf_library()")
assert_filedeps(
rule_runner, targets=["some/target"], expected={"some/target/BUILD", "some/target/f.proto"}
)
Loading

0 comments on commit d3eee59

Please sign in to comment.