wip: make e2e tests runnable outside Meta#1311
Draft
lf- wants to merge 9 commits into
Draft
Conversation
meta-cla
Bot
added
the
CLA Signed
Contributor
|
For pypi, I made this, which has been very nice: https://github.com/cormacrelf/elk No buckify step at all, just reads uv.lock. Caveat, it would work better with #1287. |
Contributor
Author
Oh that's really clever! I hadn't considered that if the buck targets are a pure function of the lock file, they can just be made by a macro. In this case it doesn't work because fb has a directory per dependency so unless we were to add more hacks to the shim to translate the names to make them all go into one directory, we would need multiple buck files. But that might be a better alternative than my vibe coded draft. As mentioned, this PR has several parts which are already mergeable on their own :) |
Instead: use the one in-tree! The problem with the one on crates.io is that it got published with a BUCK file in the source tarball, so it breaks Buck when you `reindeer vendor` it as the sources are no longer visible. The crates.io publication should certainly be fixed, but there's no reason to use the crates.io version in-repo to begin with. Fixes: facebook#1226
!! VIBE CODING WARNING !! This was aggressively clauded. It works. The code isn't quite to my standard of quality, but it's Fine. It probably needs some tests. Fortunately we have Claude for those too. ---- This package introduces a basic infrastructure for PyPI packages, necessary to make headway on running Buck2's E2E test suite outside of Meta. It's sort of modeled after Reindeer for Rust, but outputs to separate folders. On account of wanting to get something working fast, I haven't cared about building from source because that would require dealing with Python build systems. We consume wheels.
This requires two things: 1. Producing worthwhile pytest output outside of TPX: we don't have TPX to show individual test case status, so we need to enable default pytest output to get acceptable diagnostics. 2. Allowing disabling certain tests in OSS. For example, tests/core/help will always fail in OSS because the checked-in output uses a internalfb.com url for the homepage, for instance.
Quoth tests/buck_e2e.bzl:
```
if use_compiled_buck2_client_and_tpx:
base_exe = "$(location fbcode//buck2:symlinked_buck2_and_tpx)/buck2"
exe = select({
"DEFAULT": base_exe,
"ovr_config//os:windows": base_exe + ".exe",
})
else:
exe = "$(location fbcode//buck2:buck2)"
```
So this stuff needs to be accessible from other files. I would totally
believe that this is the wrong way, or that this file exists somewhere
else in fbcode than it does in OSS such that this is the wrong target
(and it's undesirable to mark it as public), but this is ultimately what
target we get at this path once we apply the transformations used in
OSS.
This is *surely* going to super blow up when this gets back into fbcode.
I am guessing this is just not exported?
Quoth tests/e2e_util/buck_workspace.py:
```
if os.environ.get("BUCK2_E2E_TEST_FLAVOR") == "isolated":
if inplace is not None:
raise Exception(
"Don't set `inplace` in `tests/core` - these tests are always isolated"
)
inplace = False
else:
if inplace is None:
raise Exception("`inplace` must be set for `buck_test()`")
```
And BUCK2_E2E_TEST_FLAVOR comes from the package value, so I think we
need a PACKAGE here.
We have to translate Python module paths because our repo root is *inside* fbcode//buck2, rather than being at fbcode//. This means that `//foo` needs to get the module path `buck2.foo`, thus this buckconfig hackery.
Wraps native.python_test to provide a pytest-based test runner. Adds the minimum stubs needed for tests/buck_e2e.bzl to load in OSS: a wrapper forwarding to the real buck_e2e.bzl in the root cell, no-op modifier and ci_hint stubs, and a remove_labels addition to the existing ci stub.
This was required for analysis. Claude stubbed it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a bunch of work comprised of small commits. If any of them look landable (they probably are!) feel free to take them and land them first (let me know if you want them in a separate PR or so).
The overall work that needed to happen to do this was:
I want feedback on the approach to Python packaging (and whether it's landable if I were to clean up the Claudeware or if you want a different approach).
I would also like to have feedback on the things I had to do because of oddball exports from fbsource: the PACKAGE file and setting
//:buck2to public.Finally I'd like advice on what to do about
buck runbehaving differently than buck test:buck runon my test target fails due to the wrong cwd for the buck-out paths it tries to run.This is caused by
inject_test_run_infogetting cell-relative paths in its environment, butpython_testgets run in the project root, I think? This doesn't affect the ExternalRunnerTestInfo because it receives absolute paths.How should I fix this? It seems quite messy and caused by the interaction of numerous pieces, none of which really intend to screw this up. I expect it works inside of Meta, so there's something weird going on in OSS which I've not quite understood.
Demo: