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

Invoke Pants via module in ITs. #8265

Merged
merged 4 commits into from
Sep 12, 2019

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Sep 9, 2019

This enables uniform execution of Pants in a subprocess whether running
in pantsbuild/pants from our custom venv, from a pants PEX or from a
pantsbuild/setup venv.

In order to support this mode, apply the PYTHONPATH scrubbing hack in
PytestRun to PythonRun and PythonRepl as well pending a fix for:
pex-tool/pex#707

This enables uniform execution of Pants in a subprocess whether running
in pantsbuild/pants from our custom venv, from a pants PEX or from a
pantsbuild/setup venv.

In order to support this mode, apply the `PYTHONPATH` scrubbing hack in
`PytestRun` to `PythonRun` and `PythonRepl` as well pending a fix for:
  pex-tool/pex#707
@stuhood
Copy link
Member

stuhood commented Sep 10, 2019

Thanks John! This didn't have any reviewers on it, and I half-acted on it anyway by adding @Eric-Arellano ... is it reviewable?

@jsirois
Copy link
Contributor Author

jsirois commented Sep 10, 2019

Thanks @stuhood, it is indeed reviewable.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Wonderful!

Also you can now add tests/python/pants_test/task:goal_options_mixin_integration to the IT whitelist :) Confirmed it works with V2 now with no additional changes necessary.

src/python/pants/BUILD Outdated Show resolved Hide resolved
src/python/pants/BUILD Outdated Show resolved Hide resolved
@jsirois jsirois merged commit 2e1037c into pantsbuild:master Sep 12, 2019
@jsirois jsirois deleted the integration_tests/invoke_module branch September 12, 2019 01:33
@@ -383,6 +356,7 @@ def run_pants_with_workdir_without_waiting(self, command, workdir, config=None,
env = os.environ.copy()
if extra_env:
env.update(extra_env)
env.update(PYTHONPATH=os.pathsep.join(sys.path))
Copy link
Member

Choose a reason for hiding this comment

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

Hey gang: I believe that this code might have caused an issue with "loose-source" plugins not ending up "all the way" on the python path when running integration tests.

The loose-source plugins are loaded with [GLOBAL] pythonpath, but the subprocessed pants runs fail with:

Exception caught: (pants.base.exceptions.BackendConfigurationError)
  File "/opt/twitter_mde/package/eepython36/68cc93925092a6fc31d254392558fa8c840f7bd5407db9435ca48ff1b8f6c07f/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/opt/twitter_mde/package/eepython36/68cc93925092a6fc31d254392558fa8c840f7bd5407db9435ca48ff1b8f6c07f/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/__main__.py", line 8, in <module>
    pants_loader.main()
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/bin/pants_loader.py", line 78, in main
    PantsLoader.run()
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/bin/pants_loader.py", line 74, in run
    cls.load_and_execute(entrypoint)
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/bin/pants_loader.py", line 67, in load_and_execute
    entrypoint_main()
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/bin/pants_exe.py", line 32, in main
    PantsRunner(start_time=start_time).run()
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/bin/pants_runner.py", line 85, in run
    options_bootstrapper=options_bootstrapper
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/bin/local_pants_runner.py", line 142, in create
    options_bootstrapper=options_bootstrapper,
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/bin/local_pants_runner.py", line 91, in parse_options
    build_config = BuildConfigInitializer.get(options_bootstrapper)
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/init/options_initializer.py", line 38, in get
    cls._cached_build_config = cls(options_bootstrapper).setup()
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/init/options_initializer.py", line 69, in setup
    self._bootstrap_options.backend_packages
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/init/options_initializer.py", line 58, in _load_plugins
    return load_backends_and_plugins(plugins, working_set, backend_packages)
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/init/extension_loader.py", line 32, in load_backends_and_plugins
    load_build_configuration_from_source(build_configuration, backends)
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/init/extension_loader.py", line 113, in load_build_configuration_from_source
    load_backend(build_configuration, backend_package)
  File "/Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/init/extension_loader.py", line 129, in load_backend
    raise BackendConfigurationError(f'Failed to load the {backend_module} backend: {e!r}')

Exception message: Failed to load the twitter.pants_internal.register backend: ModuleNotFoundError("No module named 'twitter.pants_internal.tasks.some_twitter_task'",)

Which is odd, because it sounds like maybe the register.py is located, but not some of the other code...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing comes to mind. Perhaps you can create a repro against pants own loose source plugins in a local IT? Easier, just hand-edit /Users/stuhood/src/source/.pants.d/pyprep/sources/cf3f7a2a3d17140e6302e01caf2b23a165031273/pants/init/extension_loader.py to print out sys.path and dump sys.modules to start.

Copy link
Member

@stuhood stuhood Sep 16, 2019

Choose a reason for hiding this comment

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

Holy mackerel. This was undeclared BUILD level dependencies in these tests. That is very unexpected, and very welcome... I have to go change our README to indicate that integration tests do need valid dependency lists. Exciting =)

stuhood added a commit that referenced this pull request Jul 7, 2020
### Problem

Since #8265, we have been running `PantsRunIntegrationTest`s from loose sources in the repository, which are included via `src/python/pants/testutil:int-test`'s dependency on `src/python/pants/bin:pants_local_binary`. That change thus removed the need for the `pants.pex` from our wrapper scripts.

### Solution

Remove the "`pants.pex` for integration tests" mechanism. Post #8625, pants is run from the PYTHONPATH of the test target, which will automatically include either loose sources or a `pants_requirement` target, depending on whether the test is run in or out of the pantsbuild/pants repo.

Additionally, remove one set of tests that was attempting to test that nested runs of pants do not deadlock. #7654 will eventually allow that issue to be resolved by allowing the concurrent run rather than via any sort of automatic disabling of `pantsd`, and that is much easier to test via concurrent runs _without_ nesting.

[ci skip-rust-tests]
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