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

Run all tests (with some exclusions) in github actions #9230

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

alexeyt
Copy link
Contributor

@alexeyt alexeyt commented Sep 28, 2022

Summary: Unfortunately run.php doesn't support multiple exclude files, so this diff copies the file with the smaller set of excludes used for local cmake testing and expands it. For now, if we want to mutate the list of excluded tests we will need to touch both files (but my hope is to get them to be identical and much smaller soon, which will allow us to have only one exclude file in the future).

Differential Revision: D39861976

@facebook-github-bot
Copy link
Contributor

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

alexeyt and others added 6 commits September 27, 2022 23:58
Summary: A number of tests fail in environments where we run cmake builds, apparently because the locales we use in those tests are not available. Make these tests all declare their dependency on the locales they use so that they can be skipped when the locales are not present on the system running the test.

Differential Revision: D39861973

fbshipit-source-id: 41a3d78572d42b5b707129108a488232719a0218
Summary:
I appear to have been mistaken in D39591758 (facebook@c809736) re: the crc32b hash always producing the same value for the same input. It appears that the way we implement this hash depends not on whether we're building facebook-specific extension functionality, but whether we build with buck or cmake (i.e. both default and hhvm.no_facebook=1 buck builds produce one value, and all cmake builds produce another value). The state before D39591758 (facebook@c809736) was still broken, just not in the way that I thought (the test was trying to use the presence of facebook-specific code to determine which output to expect, but that's not what the behavior change is linked to).

Since there is no good way to tell a hhvm.no_facebook=1 buck build from a cmake build, we can't correctly do what the test was originally trying to do. Instead, this diff updates the test expectatsions to accept either output.

Differential Revision: D39861974

fbshipit-source-id: 1cd053e4714d9e0552e19755e8e35a93e16c394e
Summary: We failed to update this test when we banned the ${...} interpolation syntax (because we never ran this test internally). Try to fix the test by using the currently supported syntax.

Differential Revision: D39861972

fbshipit-source-id: ecc740c2d4ebec0e151cc777eaa0107040f0e1df
Summary:
We failed to update these unit tests when we banned argument coercion for builtin arguments (because we never run these tests internally). Try to fix these tests by replacing values of the wrong type with the values that they would have been coerced to when that was still allowed.

Also fix one test expectation that was not adjusted to deal with hack array migration.

Differential Revision: D39861971

fbshipit-source-id: 7052b9ebe84d41fca4eb9eea099d991e564d1a08
Summary: For now, exclude some tests that do not work for various reasons (some that are unavoidable and some that we need to fix later).

Differential Revision: D39477972

fbshipit-source-id: 8259e1fa5fd830ba93272436b581f4f58d74dd38
Summary:
Pull Request resolved: facebook#9230

Unfortunately run.php doesn't support multiple exclude files, so this diff copies the file with the smaller set of excludes used for local cmake testing and expands it. For now, if we want to mutate the list of excluded tests we will need to touch both files (but my hope is to get them to be identical and much smaller soon, which will allow us to have only one exclude file in the future).

Differential Revision: D39861976

fbshipit-source-id: baa4f55a24f69c8aa2a8279c26cc4571fa3e8e8d
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

Hi @alexeyt!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

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.

3 participants