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

Remote execution of integration tests #8113

Open
Eric-Arellano opened this issue Jul 26, 2019 · 2 comments

Comments

@Eric-Arellano
Copy link
Contributor

commented Jul 26, 2019

#7649 and #8051 added support for remote execution of unit tests in CI. Now, we want to have remote execution of integration tests, which is where we spend the bulk of our CI time.

This will be harder because integration tests (naively) depend on the entire pants.pex and not just individual files like we have for unit tests.

One of the main decisions is whether we have every integration tests depend on the entire Pants codebase, or only the actual parts possible. Likely in the beginning, we will take the naive approach of depending on the entire pants.pex as a first prototype. The downside is that anytime anyone makes a change to any file, the cache will be invalidated for all integration tests. As a followup, we could de-monotholize Pants to work around this.

Possible steps to land this:

  • Run all tests with --chroot, which will catch several issues with implicit dependencies on being in the actual buildroot and better reflects how V2 works. #7281 and #8084.
  • Figure out how to get integration tests to depend on having pants.pex available to them. This may be tricky because that is a generated file. Some prior art is how we depend on native_engine.so, which is also a generated file:
    resources(
    name='native_engine_shared_library',
    sources=['native_engine.so']
    • How should we ensure that this file is not out of date? Especially when testing locally, this is a likely issue, that the integration test is using an outdated version of pants.pex. This may also make local integration testing much happier if it's going to require regenerating the PEX every time!
  • Get integration tests working for tests/python/pants_test/help:help_integration, both locally with V2 and with remoting.

We will almost certainly use a whitelist approach to remoting tests in CI. Even if we can only remote 1-5 integration tests in CI at first, we should land that because it establishes the infrastructure needed to port more tests.

@stuhood

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

Likely in the beginning, we will take the naive approach of depending on the entire pants.pex as a first prototype.

Yea, we should. As tempted as I am to suggest that we should change this at the same time, let's try to avoid thinking about it, and just swap from throwing money at Travis to throwing it at RBE (where at least we think that we can get more parallelism). We can worry about reducing the amount of money next.

@stuhood

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

We will almost certainly use a whitelist approach to remoting tests in CI. Even if we can only remote 1-5 integration tests in CI at first, we should land that because it establishes the infrastructure needed to port more tests.

So, in theory there is no need to have separate integration and unit test shards. And retries should be (relatively) cheap, because retrying should mostly hit the RBE/remoting cache.

Eric-Arellano added a commit that referenced this issue Aug 25, 2019
Properly strip source root prefixes for V2 Pytest runner (#8185)
### Problem
#7696 introduced support for source roots to V2 Pytest for the first time, but it did not support loose files. #8063 tried to fix this by no longer stripping the source root from source files. However, this caused two issues:

1) Regression that `repr()` now shows the full path, not the relativized path: #8063 (comment)
2) Namespace packages, such as contrib code, do not work with V2. A namespace package is where multiple folders have the same package name, like `pants`, but may not within the same actual folder.
   * Contrib unit tests would fail because `PYTHONPATH` had two entries referring to `pants`: `src/python/pants` and `contrib/mypy/src/python/pants`. Python would use whichever entry came first and ignore the other, which does not work. Instead, we want those two to merge into one namespace package.
 
### Solution
Follow the V1 Pytest approach of stripping the source root, like we used to, but only for source files. Loose files (i.e. `files()`) are not stripped so that the file system APIs work as expected. This approach comes from:

https://github.com/pantsbuild/pants/blob/d048fd2e8f34adc32fdce36a51a765fbf6067cff/src/python/pants/backend/python/subsystems/pex_build_util.py#L101-L110

### Result
V2 Pytest now supports both source roots and loose files! This allows us to run most contrib unit tests with V2 and unblocks #8113.

Because over 99% of unit tests are now remoted, we go back to only one CI shard for unit tests.
Eric-Arellano added a commit that referenced this issue Sep 2, 2019
Use `./pants.pex`, not `./pants`, to run internal integration tests (#…
…8183)

## Changes to get `--chroot` working
As prework to running integration tests with `--chroot` and then [remoting](#8113), we first must make two major changes to how we handle integration tests:

1) Having integration tests depend on `//:build_root`. This ensures that `get_buildroot()` properly resolves to the chrooted folder in `.pants.d`, rather than traversing all the way up to the original build root.
2) Using `./pants.pex` rather than `./pants` in integration tests. Because V1 strips the prefix of source roots, whereby `src/python/pants` becomes `pants`, we cannot also have a file named `pants` in the same build root. Instead, we can safely depend on `./pants.pex`.

## Autogeneration of `pants.pex`
Now that we depend on `pants.pex`, it is very easy for this file to fall out-of-date and to unintentionally be testing old code.

We work around this via the design in #8209. First, we start caching built PEXes in `~/.cache/pants`, with the ID derived from all the source files used for Pants. This allows us to quickly switch between branches without having to regenerate the PEX every time.

Then, we teach `./pants` to run `bootstrap_pants_pex.sh` whenever `test` is one of the goals and it's not CI. This will ensure that the file is always up-to-date.
Eric-Arellano added a commit that referenced this issue Sep 7, 2019
Run first few integration tests through remote execution (#8210)
Part of #8113.

This unifies our handling of whitelist and blacklists for CI for unit tests and integration tests, making it easier to understand how targets are put into one of the 4 buckets: `v1_no_chroot`, `v1_chroot`, `v2_local`, and `v2_remote`, where `v1_no_chroot` is the least far along in the migration and `v2_remote` is the end goal.
Eric-Arellano added a commit that referenced this issue Sep 7, 2019
Properly depend on plugins in chrooted integration tests (#8257)
### Problem

Most integration tests end up with `pants.ini` in their build root due to the dependency in `pants_test/option/util/fakes.py`:

https://github.com/pantsbuild/pants/blob/44128e8c9a10d20ed5988d08188aa51354a79568/tests/python/pants_test/option/util/BUILD#L4-L10

When this happens, `./pants.pex` will try to apply all the options in `pants.ini`, including loading all of the backend packages:

https://github.com/pantsbuild/pants/blob/44128e8c9a10d20ed5988d08188aa51354a79568/pants.ini#L47-L69

Most of these backend packages had no explicit dependencies declared via BUILD entries, so when most ITs were run with `--chroot` or `--v2`, they would fail to load the backend packages.

### Solution

Explicitly declare all plugin requirements as a part of the `pants_local_binary` target, which is what's used to bootstrap `pants.pex`.

Note that these must be declared as a part of `pants_local_binary`, rather than simply in the target `:int-test`, because the generated file needs to have all of the code necessary to run properly directly in the PEX file.

### Result

Further progress on #8113. Now the main remaining issue for remoting the majority of integration tests appears to be making the implicit dependencies on `examples` and `testprojects` explicit, which this PR makes an initial example of how this may be done.
Eric-Arellano added a commit that referenced this issue Sep 8, 2019
Run all contrib integration tests using V2 remote execution (#8261)
Requires redesigning the MyPy integration test. It was originally testing against itself. Now it has a proper `contrib/examples` folder like everything else.

Brings integration tests to 25% remoted (#8113).
Eric-Arellano added a commit that referenced this issue Sep 8, 2019
Port ~20 integration tests to remote execution (#8262)
Brings integration tests to 38% remoted (#8113).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.