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

Avoid hard-failing if buck2 unavailable #853

Closed
wants to merge 6 commits into from
Closed

Conversation

stroxler
Copy link
Contributor

Summary:
We hit this code path in OSS CI. In the future it will be on
people writing upgrade code to make sure the logic works okay
in CI, or else isn't executed by github CI.

But for now I am trying to get a quick fix, because trunk is
broken and it's spamming us with tasks.

Differential Revision: D57360862

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57360862

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57360862

stroxler added a commit that referenced this pull request May 14, 2024
Summary:
Pull Request resolved: #853

We hit this code path in OSS CI. In the future it will be on
people writing upgrade code to make sure the logic works okay
in CI, or else isn't executed by github CI.

But for now I am trying to get a quick fix, because trunk is
broken and it's spamming us with tasks.

Differential Revision: D57360862
Summary:
Our uses of `patch(...)` are broken for open-source, because
in our current open-source CI our `__name__` involves `pyre-check`,
which is not legal to use.

As a result, for patching anything internal we need to use
`patch.object` instead.

It's okay to use `patch(...)` for stdlib, that doesn't cause
problems (although I do wish we didn't have to rely on patching
quite so much!)

I may break this into a couple commits to keep them small.

Differential Revision: D57358329
Summary:

This is the second of a few commits removing the use of
`unittest.patch` on pyre-internal functions and classes;
instead we need to always use `patch.object` to avoid getting
errors on invalid names in github CI.

Differential Revision: D57359461
Summary:

I have to stack a separate commit because there's a closed
PR that the bot won't seem to let me reopen, and I need
to test these changes on github :/

Differential Revision: D57359996
Summary:
`Path.is_relative_to` was added after 3.8, which we still need
to support in open-source. As a result we need to fix this
to avoid test failures in github CI.

Differential Revision: D57360861
Summary:
It's a little unfortunate to disable a unit test that passes
when run using buck, but we really can't have tests checked
in that break github CI.

We need to figure out why this breaks, and either fix it or
find a way to run it only locally, I filed T189132787 to
look at this. In the meantime though I think the best option
is to disable the test because folks are getting spammed with
github CI failure notices.

Differential Revision: D57361655
Summary:

We hit this code path in OSS CI. In the future it will be on
people writing upgrade code to make sure the logic works okay
in CI, or else isn't executed by github CI.

But for now I am trying to get a quick fix, because trunk is
broken and it's spamming us with tasks.

Differential Revision: D57360862
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D57360862

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in fba3b7e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants