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

Factor up shared test partitioning code. #2

Closed
wants to merge 1 commit into from

Conversation

jsirois
Copy link
Owner

@jsirois jsirois commented Feb 1, 2018

This factors the handling of --fast/--no-fast partitioning as well
as the attendant results summarization and caching and invalidation
handling to a mixin that PytestRun and JUnitRun share.

Fixes pantsbuild#5307

This factors the handling of `--fast`/`--no-fast` partitioning as well
as the attendant results summarization and caching and invalidation
handling to a mixin that `PytestRun` and `JUnitRun` share.

Fixes pantsbuild#5307
@jsirois jsirois closed this Feb 1, 2018
jsirois pushed a commit that referenced this pull request Jul 6, 2019
Per pantsbuild#7649, we are working to remote unit tests in CI.

This implements step #2, which allows us to run unit tests locally (several, but not all tests). Here, we set up `pants.remote.ini` so that users only must point to it and provide the authentication token.
jsirois pushed a commit that referenced this pull request Dec 14, 2019
### Problem

MyPy does not understand `Goal.Options`, e.g. `List.Options` and `Fmt.Options`, because the implementation relies on runtime creation of the class. MyPy only can understand classes declared at compile time. This is the same reason we needed to rework `pants.util.objects.enum`, `pants.util.objects.datatype`, and `pants.engine.objects.Collection`.

At the same time, we want to preserve as much of the original API as possible because it provided a useful coupling between the `Goal` class and its underlying subsystem. We need to preserve features like the goal's docstring populating `./pants help`, for example.

One of the challenges is that the original implementation requires the Subsystem inner class to have knowledge of the Goal outer class. This is not a well-supported idiom in Python: https://stackoverflow.com/questions/2024566/access-outer-class-from-inner-class-in-python.

### Attempted solutions

#### Attempt #1: pass outer Goal directly to inner Options

I started by trying to implement https://stackoverflow.com/a/7034206. The issue is that we make heavy use of `@classmethod` and `@classproperty`, so we need to be able to access `outer_cls` at the `cls` scope and this pattern only allows using it at the instance scope (`self`).

#### Attempt #2: `Goal.Options` directly instantiated with reference to `Goal`

I then tried this pattern:

```python
class List(Goal):
  name = "list"
  
  @classmethod
  def register_options(cls, register):
    ...

class ListOptions(Options):
  goal_cls = List

@rule 
def list(..., options: ListOptions) -> List:
   ...
```

This required rewriting `rules.py` to ensure that the `Options` were properly registered with the rule graph:

https://github.com/pantsbuild/pants/blob/b88dc0f7d9c0ce322714b74d78f312b4c4b847a2/src/python/pants/engine/rules.py#L138-L155

This worked fine when a rule explicitly requested the `Options` in its rule params, but it meant that rules without explicitly registered options, like `fmt` and `lint`, were no longer registered with the engine! There was no way to feed in the sibling `FmtOptions` or `LintOptions` because these would have to be explicitly created and somehow passed to the engine, even though the corresponding rules had no need to directly consume those subsystems.

### Solution: new `GoalSubsystem` class

Setting up a `Goal` will now involve two steps:
1. Register a `GoalSubsystem` that has the name of the goal, the docstring, and any options (if any).
1. Register a `Goal`, which only needs to point `subsystem_cls` to the sibling `GoalSubsystem`.

For example:

```python
class ListOptions(GoalSubsystem):
  """List all the targets."""
  name = "list"

  @classmethod
  def register_options(cls, register):
    ...

class List(Goal):
  subsystem_cls = ListOptions
```

Rules may then explicitly request the `GoalSubsystem`, e.g. `ListOptions`.

Consumers of the `Goal`, like `rules.py` and `engine_initializer.py`, simply access `Goal.subsystem_cls` to get the attributes they previously accessed directly.

#### Conceptual motivation

This strengthens the concept that Subsystems are the sole mechanism for consuming options in V2. The `GoalSubsystem` may be seen as the external API—containing all the options, the goal name, and the goal description seen by the outside world. Meanwhile, the `Goal` consumes that subsystem (just like the `black.fmt` rule consumes the `Black` subsystem) to produce a new type used internally by the engine.

### Result

MyPy now understands V2 goals, which unblocks us from using MyPy with V2 code and tests (`TestBase` and `PantsRunIntegration` are blocked by this issue).
jsirois pushed a commit that referenced this pull request Apr 9, 2020
…uild#9466)

Twice now, we've had `Sources` fields that require a specific number of files. So, this PR formalizes a mechanism to do this.

Error message #1:

> ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have 3 files, but it had 1 file.

Error message #2: 

> ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have 2 or 3 files, but it had 1 file.

Error message pantsbuild#3: 

> ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have a number of files in the range `range(20, 20000, 10)`, but it had 1 file.


[ci skip-rust-tests]  # No Rust changes made.
[ci skip-jvm-tests]  # No JVM changes made.
jsirois pushed a commit that referenced this pull request May 3, 2020
## Goals of design

See https://docs.google.com/document/d/1tJ1SL3URSXUWlrN-GJ1fA1M4jm8zqcaodBWghBWrRWM/edit?ts=5ea310fd for more info. 

tl;dr:

1) Protocols now only have one generic target, like `avro_library`. This means that call sites must declare which language should be generated from that protocol.
    * Must be declarative.
2) You can still get the original protocol sources, e.g. for `./pants filedeps`.
3) Must work with subclassing of fields.
4) Must be extensible.
     * Example: Pants only implements Thrift -> Python. A plugin author should be able to add Thrift -> Java.

## Implementation

Normally, to hydrate sources, we call `await Get[HydratedSources](HydrateSourcesRequest(my_sources_field))`. We always use the exact same rule to do this because all `sources` fields are hydrated identically.

Here, each codegen rule is unique. So, we need to use unions. This means that we also need a uniform product for each codegen rule for the union to work properly. This leads to:

```python
await Get[GeneratedSources](GenerateSourcesRequest, GeneratePythonFromAvroRequest(..))
await Get[GeneratedSources](GenerateSourcesRequest, GenerateJavaFromThriftRequest(..))
```

Each `GenerateSourcesRequest` subclass gets registered as a union rule. This achieves goal pantsbuild#4 of extensibility.

--

To still work with subclassing of fields (goal pantsbuild#3), each `GenerateSourcesRequest` declares the input type and output type, which then allows us to use `isinstance()` to accommodate subclasses:

```python
class GenerateFortranFromAvroRequest(GenerateSourcesRequest):
    input = AvroSources
    output = FortranSources
```

--

To achieve goals #1 and #2 of allowing call sites to declaratively either get the original protocol sources or generated sources, we hook up codegen to the `hydrate_sources` rule and `HydrateSourcesRequest` type:

```python
protocol_sources = await Get[HydratedSources](HydrateSourcesRequest(avro_sources, for_sources_types=[FortranSources], codegen_enabled=True))
```

[ci skip-rust-tests]
[ci skip-jvm-tests]
jsirois pushed a commit that referenced this pull request Jul 24, 2020
Before:
```
▶ ./pants dependencies src/python/pants/engine/internals:native
3rdparty/python:typing-extensions
src/python/pants/base:exiter.py
src/python/pants/engine/internals:__init__.py
src/python/pants/engine/internals:native_engine
src/python/pants/engine:fs.py
src/python/pants/engine:selectors.py
src/python/pants/engine:unions.py
src/python/pants/util:memo.py
src/python/pants/util:meta.py
```

After:
```
▶ ./pants dependencies src/python/pants/engine/internals:native
3rdparty/python:typing-extensions
src/python/pants/base/exiter.py
src/python/pants/engine/fs.py
src/python/pants/engine/internals/__init__.py
src/python/pants/engine/internals:native_engine
src/python/pants/engine/selectors.py
src/python/pants/engine/unions.py
src/python/pants/util/memo.py
src/python/pants/util/meta.py
```

This change solves several key questions with file args, as explained in https://docs.google.com/document/d/14MAst0Q0HpoLqysiN7KoKDkA6-Ga_KhjICTl0EC16uI/edit#heading=h.y495n7exlx1x:

1. Alignment with file arguments, which will use literal file names and will always result in generated subtargets being used.
2. A syntax for explicitly declaring generated subtarget dependencies in BUILD files.
     * Without this change, there is ambiguity between whether you intended :example.txt to mean a literal target or a generated subtarget.
3. Thanks to #2, a way to explicitly ignore false positives via `!` in the dependencies field of BUILD files.

For alignment with target addresses and the `sources` field, we will plan to allow relative references to generated subtargets. `foo.py` will be relative to the BUILD file, and you need `//foo.py` to mean relative to the build root. This does not work with subdirectories. `subdir/a.py` is relative to the build root.

### Multiple owners
If there are multiple owners of a target, and we generate a subtarget over the same file for each owner, then the resulting addresses would all "look" the same to the user because they would have the same `Address.spec`. This is confusing to the user.

For now, we defer this problem by never using generated subtargets when there are multiple owners. In the feature, we could add a syntax to refer to the original owner when there's ambiguity, e.g. `helloworld/app.py@:original_target`. This is safe because of the following semantics with multiple owners:
- dep inference -> no-op
- explicit BUILD files -> error
- CLI file args -> don't generate subtargets, and instead use all the original owners.

[ci skip-rust-tests]
jsirois pushed a commit that referenced this pull request Aug 16, 2020
…0538)

Before:

```
▶ ./pants lint src/python/pants/util/strutil.py
09:21:31.63 [INFO] Completed: Run Docformatter on 1 target: src/python/pants/util/strutil.py.
09:21:31.63 [INFO] Completed: Run isort on 1 target: src/python/pants/util/strutil.py.
09:21:31.76 [INFO] Completed: Run Flake8 on 1 target: src/python/pants/util/strutil.py
09:21:34.59 [INFO] Completed: Run Black on 1 target: src/python/pants/util/strutil.py.
𐄂 Black failed.
would reformat src/python/pants/util/strutil.py
Oh no! 💥 💔 💥
1 file would be reformatted.


✓ Docformatter succeeded.

✓ Flake8 succeeded.

✓ isort succeeded.
/Users/eric/.pex/installed_wheels/fe0462426518d1eb662d92fd867eac898630585e/setuptools-49.2.0-py3-none-any.whl/setuptools/distutils_patch.py:26: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.
  "Distutils was imported before Setuptools. This usage is discouraged "


```

After:

```
▶ ./pants lint src/python/pants/util/strutil.py
09:10:02.67 [INFO] Completed: Lint with docformatter - succeeded.
09:10:02.79 [INFO] Completed: Lint with isort - succeeded.
/Users/eric/.pex/installed_wheels/fe0462426518d1eb662d92fd867eac898630585e/setuptools-49.2.0-py3-none-any.whl/setuptools/distutils_patch.py:26: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.
  "Distutils was imported before Setuptools. This usage is discouraged "

09:10:02.92 [WARN] Completed: Lint with Flake8 - failed.
src/python/pants/util/strutil.py:28:5: E125 continuation line with same indent as next logical line

09:10:05.67 [WARN] Completed: Lint with Black - failed.
would reformat src/python/pants/util/strutil.py
Oh no! 💥 💔 💥
1 file would be reformatted.


𐄂 Black failed.
✓ Docformatter succeeded.
𐄂 Flake8 failed.
✓ isort succeeded.
```

We also now say when a linter was skipped.

```
▶ ./pants lint src/python/pants/util/strutil.py --black-skip --isort-skip --no-pantsd
09:10:41.66 [INFO] Completed: Lint with docformatter - succeeded.
09:10:41.66 [WARN] Completed: Lint with Flake8 - failed.
src/python/pants/util/strutil.py:28:5: E125 continuation line with same indent as next logical line


- Black skipped.
✓ Docformatter succeeded.
𐄂 Flake8 failed.
- isort skipped.
```

If a linter partitions, e.g. by interpreter constraints, the output will look like:

```
▶ ./pants lint src/python/pants/util:{t1,t2}
▶ ./pants --backend-packages=pants.backend.python.lint.pylint lint src/python/pants/util:t{1,2}
07:34:25.21 [INFO] Completed: Lint with Flake8 - succeeded.
Partition #1 - ['CPython==3.7.*']:

Partition #2 - ['CPython==3.6.*']:

07:34:25.21 [INFO] Completed: Lint with docformatter - succeeded.
07:34:25.22 [INFO] Completed: Lint with isort - succeeded.
/Users/eric/.pex/installed_wheels/fe0462426518d1eb662d92fd867eac898630585e/setuptools-49.2.0-py3-none-any.whl/setuptools/distutils_patch.py:26: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.
  "Distutils was imported before Setuptools. This usage is discouraged "

07:34:25.22 [INFO] Completed: Lint with Black - succeeded.
All done! ✨ 🍰 ✨
2 files would be left unchanged.

07:34:30.29 [WARN] Completed: Lint using Pylint - failed (exit code 20).
Partition #1 - ['CPython>=3.6,==3.7.*']:
************* Module pants.util.strutil_test
src/python/pants/util/strutil_test.py:16:2: W0511: TODO(Eric Ayers): Backfill tests for other methods in strutil.py (fixme)
src/python/pants/util/strutil_test.py:53:0: C0301: Line too long (129/100) (line-too-long)
....

------------------------------------------------------------------
Your code has been rated at 7.22/10 (previous run: 7.22/10, +0.00)

Partition #2 - ['CPython>=3.6,==3.6.*']:
************* Module pants.util.dirutil_test
src/python/pants/util/dirutil_test.py:84:0: C0330: Wrong hanging indentation before block (add 4 spaces).
        self, tempfile_mkdtemp, dirutil_safe_rmtree, os_getpid, atexit_register
        ^   | (bad-continuation)
...

------------------------------------------------------------------
Your code has been rated at 7.14/10 (previous run: 7.14/10, +0.00)


✓ Black succeeded.
✓ Docformatter succeeded.
✓ Flake8 succeeded.
𐄂 Pylint failed.
✓ isort succeeded.
```

With `--per-target-caching`, it will look like:

```
▶ ./pants lint src/python/pants/util/{dir,str}util.py --lint-per-target-caching
09:12:23.10 [INFO] Completed: Lint with docformatter - succeeded.
09:12:23.10 [INFO] Completed: Lint with isort - succeeded.
/Users/eric/.pex/installed_wheels/fe0462426518d1eb662d92fd867eac898630585e/setuptools-49.2.0-py3-none-any.whl/setuptools/distutils_patch.py:26: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.
  "Distutils was imported before Setuptools. This usage is discouraged "

09:12:23.10 [WARN] Completed: Lint with Black - failed.
would reformat src/python/pants/util/strutil.py
Oh no! 💥 💔 💥
1 file would be reformatted.

09:12:23.10 [WARN] Completed: Lint with Flake8 - failed.
src/python/pants/util/strutil.py:28:5: E125 continuation line with same indent as next logical line

09:12:23.10 [INFO] Completed: Lint with Black - succeeded.
All done! ✨ 🍰 ✨
1 file would be left unchanged.

09:12:23.10 [INFO] Completed: Lint with docformatter - succeeded.
09:12:23.10 [INFO] Completed: Lint with isort - succeeded.
/Users/eric/.pex/installed_wheels/fe0462426518d1eb662d92fd867eac898630585e/setuptools-49.2.0-py3-none-any.whl/setuptools/distutils_patch.py:26: UserWarning: Distutils was imported before Setuptools. This usage is discouraged and may exhibit undesirable behaviors or errors. Please use Setuptools' objects directly or at least import Setuptools first.
  "Distutils was imported before Setuptools. This usage is discouraged "

09:12:23.95 [INFO] Completed: Lint with Flake8 - succeeded.

𐄂 Black failed.
✓ Docformatter succeeded.
𐄂 Flake8 failed.
✓ isort succeeded.
```

[ci skip-rust]
[ci skip-build-wheels]
jsirois pushed a commit that referenced this pull request Sep 11, 2020
…10728)

Since 2016, users have been asking for a way to install wheels from a URL, e.g. from Git.

There are two approaches to VCS-style requirements:

1) Pip's proprietary syntax, e.g. `git+https://github.com/pypa/pip.git#egg=pip`. This is what most people are familiar with. https://pip.pypa.io/en/stable/reference/pip_install/#vcs-support
2) PEP 440's standardized "direct references", e.g. `pip@ git+https://github.com/pypa/pip.git`.

Turns out, ever since we upgraded to Pex 2.0, approach #2 has worked to achieve this goal. But few users knew about it (including us), and approach #1 has continued to not work.

We use setuptools to parse our requirements before passing to Pex. Why? We need to normalize it to, for example, strip comments from `requirements.txt`; and because we need the parsed information to do things like create a module mapping for dependency inference. However, `pkg_resources.Requirement.parse()` chokes on Pip's VCS style, and only understands PEP 440.

Both the Pip VCS and PEP 440 approaches achieve the same end goal. We could use some custom libraries like https://github.com/sarugaku/requirementslib and https://github.com/davidfischer/requirements-parser to allow for Pip's proprietary format, but that adds new complexity to our project.

Nevertheless, few users are familiar with PEP 440, so we eagerly detect if they're trying to use the Pip style and have a nice error message for how to instead use PEP 440:

```
▶ ./pants list 3rdparty/python: --no-print-exception-stacktrace
13:25:59.67 [ERROR] 1 Exception encountered:

  MappingError: Failed to parse 3rdparty/python/BUILD:
Invalid requirement 'git+https://github.com/django/django.git#egg=django' in 3rdparty/python/requirements.txt at line 32: Parse error at "'+https:/'": Expected stringEnd

It looks like you're trying to use a pip VCS-style requirement?
Instead, use a direct reference (PEP 440).

Instead of this style:

    git+https://github.com/django/django.git#egg=django
    git+https://github.com/django/django.git@stable/2.1.x#egg=django
    git+https://github.com/django/django.git@fd209f62f1d83233cc634443cfac5ee4328d98b8#egg=django

Use this style, where the first value is the name of the dependency:

    Django@ git+https://github.com/django/django.git
    Django@ git+https://github.com/django/django.git@stable/2.1.x
    Django@ git+https://github.com/django/django.git@fd209f62f1d83233cc634443cfac5ee4328d98b8
```

Closes pantsbuild#3063.

### Additional benefit: doesn't allow `--editable`

With Pip VCS-style requirements, it's common to use `-e` or `--editable`, which changes how the distribution is downloaded so that the user can make edits to it. That is not sensible in Pants, and we are unlikely to ever support this feature.

PEP 440 style does not support `-e`, so, when users migrate to PEP 440, they won't have the opportunity to inadvertently use `-e` and for it to not work like they expect.

[ci skip-rust]
[ci skip-build-wheels]
jsirois added a commit that referenced this pull request Feb 23, 2021
This speeds things up:
```
$ hyperfine --warmup 2 './dist/pants.2.3.0.dev3+gitddc75f42..pex -V' '../pants/dist/pants.2.3.0.dev3+gitddc75f42..pex -V'
Benchmark #1: ./dist/pants.2.3.0.dev3+gitddc75f42..pex -V
  Time (mean ± σ):     636.7 ms ±  23.3 ms    [User: 441.7 ms, System: 35.9 ms]
  Range (min … max):   618.9 ms … 694.6 ms    10 runs

Benchmark #2: ../pants/dist/pants.2.3.0.dev3+gitddc75f42..pex -V
  Time (mean ± σ):      1.170 s ±  0.017 s    [User: 959.8 ms, System: 56.2 ms]
  Range (min … max):    1.155 s …  1.212 s    10 runs

Summary
  './dist/pants.2.3.0.dev3+gitddc75f42..pex -V' ran
    1.84 ± 0.07 times faster than '../pants/dist/pants.2.3.0.dev3+gitddc75f42..pex -V'
```

[ci skip-rust]
jsirois added a commit that referenced this pull request Feb 24, 2021
This speeds things up:
```
$ hyperfine --warmup 2 './dist/pants.2.3.0.dev3+gitddc75f42..pex -V' '../pants/dist/pants.2.3.0.dev3+gitddc75f42..pex -V'
Benchmark #1: ./dist/pants.2.3.0.dev3+gitddc75f42..pex -V
  Time (mean ± σ):     636.7 ms ±  23.3 ms    [User: 441.7 ms, System: 35.9 ms]
  Range (min … max):   618.9 ms … 694.6 ms    10 runs

Benchmark #2: ../pants/dist/pants.2.3.0.dev3+gitddc75f42..pex -V
  Time (mean ± σ):      1.170 s ±  0.017 s    [User: 959.8 ms, System: 56.2 ms]
  Range (min … max):    1.155 s …  1.212 s    10 runs

Summary
  './dist/pants.2.3.0.dev3+gitddc75f42..pex -V' ran
    1.84 ± 0.07 times faster than '../pants/dist/pants.2.3.0.dev3+gitddc75f42..pex -V'
```
jsirois added a commit that referenced this pull request May 12, 2021
# 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]
jsirois pushed a commit that referenced this pull request Oct 30, 2021
This lets you compile a package without needing to run tests or package a binary. 

Note that you can only directly compile a first-party package - to check if a third-party package is buildable, it must be a dependency of a first-party package. Works around pantsbuild#13175.

This does not yet have an optimal UX. It's overly chatty:

```
❯ ./pants check testprojects/src/go::
13:36:38.57 [INFO] Initializing scheduler...
13:36:39.20 [INFO] Scheduler initialized.
13:36:39.71 [ERROR] Completed: pants.backend.go.goals.check.check_go - go failed (exit code 1).
Partition #1 - github.com/pantsbuild/pants/testprojects/src/go/pants_test:
./testprojects/src/go/pants_test/foo.go:3:1: syntax error: non-declaration statement outside function body

Partition #2 - github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar:



𐄂 go failed.
```

We also should be streaming the result of each compilation as it happens, which has an added benefit of that output happening when compilation happens as a side effect of `run`, `package`, and `test`. Those fixes will be in a followup.

[ci skip-rust]
[ci skip-build-wheels]
jsirois pushed a commit that referenced this pull request Oct 30, 2021
Before:

```
❯ ./pants check testprojects/src/go::
14:51:39.15 [INFO] Completed: pants.backend.go.goals.check.check_go - go succeeded.
Partition #1 - github.com/pantsbuild/pants/testprojects/src/go/pants_test:

Partition #2 - github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar:



✓ go succeeded.
```
```
❯ ./pants check testprojects/src/go::
14:52:54.54 [ERROR] Completed: pants.backend.go.goals.check.check_go - go failed (exit code 1).
Partition #1 - github.com/pantsbuild/pants/testprojects/src/go/pants_test:
./testprojects/src/go/pants_test/foo.go:3:1: syntax error: non-declaration statement outside function body

Partition #2 - github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar:



𐄂 go failed.
```

After:

```
❯ ./pants check testprojects/src/go::
14:20:40.79 [INFO] Completed: Check Go compilation - go succeeded.

✓ go compile succeeded.
```

```
❯ ./pants check testprojects/src/go::
14:23:16.18 [ERROR] Completed: Compile with Go - github.com/pantsbuild/pants/testprojects/src/go/pants_test failed (exit code 1).
./testprojects/src/go/pants_test/foo.go:3:1: syntax error: non-declaration statement outside function body

14:23:16.18 [ERROR] Completed: Check Go compilation - go failed (exit code 1).

𐄂 go compile failed.
```

That is, we now:

- Stream results for each package, rather than waiting to batch at the very end.
- Don't log the `Partition #1` part, which is verbose and confusing.
- Log success at DEBUG level, rather than INFO level, for less noise. (Reminder: these workunits will show up in the dynamic UI still)

[ci skip-rust]
jsirois pushed a commit that referenced this pull request Nov 2, 2021
…uild#13379) (pantsbuild#13388)

Before:

```
❯ ./pants check testprojects/src/go::
14:51:39.15 [INFO] Completed: pants.backend.go.goals.check.check_go - go succeeded.
Partition #1 - github.com/pantsbuild/pants/testprojects/src/go/pants_test:

Partition #2 - github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar:



✓ go succeeded.
```
```
❯ ./pants check testprojects/src/go::
14:52:54.54 [ERROR] Completed: pants.backend.go.goals.check.check_go - go failed (exit code 1).
Partition #1 - github.com/pantsbuild/pants/testprojects/src/go/pants_test:
./testprojects/src/go/pants_test/foo.go:3:1: syntax error: non-declaration statement outside function body

Partition #2 - github.com/pantsbuild/pants/testprojects/src/go/pants_test/bar:



𐄂 go failed.
```

After:

```
❯ ./pants check testprojects/src/go::
14:20:40.79 [INFO] Completed: Check Go compilation - go succeeded.

✓ go compile succeeded.
```

```
❯ ./pants check testprojects/src/go::
14:23:16.18 [ERROR] Completed: Compile with Go - github.com/pantsbuild/pants/testprojects/src/go/pants_test failed (exit code 1).
./testprojects/src/go/pants_test/foo.go:3:1: syntax error: non-declaration statement outside function body

14:23:16.18 [ERROR] Completed: Check Go compilation - go failed (exit code 1).

𐄂 go compile failed.
```

That is, we now:

- Stream results for each package, rather than waiting to batch at the very end.
- Don't log the `Partition #1` part, which is verbose and confusing.
- Log success at DEBUG level, rather than INFO level, for less noise. (Reminder: these workunits will show up in the dynamic UI still)

[ci skip-rust]
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
jsirois pushed a commit that referenced this pull request May 13, 2022
…tsbuild#15271)

Add a `jvm_lockfile` pytest fixture and related custom mark `pytest.mark.jvm_lockfile` for defining lockfiles for use in tests. This allows avoiding having to replicate lockfile content in code using `TestCoursierWrapper`, which can become unwiedly once a resolve in complicated enough.

Lockfiles are generated by running `./pants internal-generate-test-lockfile-fixtures ::` which collects all tests marked with `pytest.mark.jvm_lockfile`, resolves their requirements, and writes out the resulting lockfiles. A custom Pants goal is used so that the plugin code gets access to the Pants source tree and targets. (An earlier iteration of this PR tried to use a `RuleRunner` in a script under `build-support/bin`, but `RuleRunner` is used for testing in an isolated directory and has no access to the Pants repository sources.)
jsirois pushed a commit that referenced this pull request May 13, 2023
The Pants native client which was introduced in pantsbuild#11922 has so far only
been usable in the Pants repo when the `USE_NATIVE_PANTS` environment
variable was set.

To make the native client available to end users, this change begins
distributing the native-client binary in Pants wheels. A corresponding
change in the `scie-pants` repo
(pantsbuild/scie-pants#172) uses the native
client to run `pants`.

To reduce the surface area of `scie-pants` (rather than having it be
responsible for handling the special `75` exit code similar to the
`pants` script integration), this PR also moves to having the
native-client execute its own fallback (via `execv`) to the Pants
entrypoint. In future, the `pantsbuild/pants` `pants` script could also
use that facility (e.g. by specifying a separate `pants_server` bash
script as the entrypoint to use as the `_PANTS_SERVER_EXE`).

----

As originally demonstrated on pantsbuild#11831, the native client is still much
faster than the legacy client. Using
pantsbuild/scie-pants#172, the timings look
like:
```
Benchmark #1: PANTS_NO_NATIVE_CLIENT=true PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help
  Time (mean ± σ):      1.161 s ±  0.067 s    [User: 830.6 ms, System: 79.2 ms]
  Range (min … max):    1.054 s …  1.309 s    10 runs

Benchmark #2: PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help
  Time (mean ± σ):     271.0 ms ±  30.6 ms    [User: 8.9 ms, System: 6.9 ms]
  Range (min … max):   241.5 ms … 360.6 ms    12 runs

Summary
  'PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help' ran
    4.29 ± 0.54 times faster than 'PANTS_NO_NATIVE_CLIENT=true PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help'
```

Fixes pantsbuild#11831.
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.

1 participant