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

Fix pyre command for type inference provider #523

Merged
merged 8 commits into from
Sep 16, 2021

Conversation

rodrigozhou
Copy link
Contributor

Summary

Pyre command for type inference provider has one of its params quoted and causing the Pyre to fail to parse the command line.

Test Plan

Added unit test to generate the cache and compare with the expected result.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 7, 2021
@zsol
Copy link
Member

zsol commented Sep 8, 2021

Looks like the new test is failing because pyre is not started. Might need to wrap the test code in a context manager or decorator that runs pyre start --no-watchman and tears it down afterwards (pyre stop)

@zsol
Copy link
Member

zsol commented Sep 8, 2021

cc @stroxler if the quotes are causing a problem, how are you not running into it?

@rodrigozhou
Copy link
Contributor Author

@zsol Updated to start and stop Pyre in test setUp and tearDown functions.

@stroxler
Copy link
Contributor

My changes bumped the pyre version which did seem to trigger a pyre start somewhere because I had to add the --no-watchman flag, but it sounds like we probably didn't start pyre pointed at the test code (my guess is that we were starting it up pointed at LibCST itself)

The error we're getting makes me think we lack a .pyre_configuration json file in the test fixture.

Assuming I'm correct about this being the problem, we could fix that by manually adding one, or probably (according to https://pyre-check.org/docs/getting-started#setting-up-a-project) by running pyre init prior to pyre start.

Copy link
Member

@zsol zsol left a comment

Choose a reason for hiding this comment

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

lets go

@zsol zsol merged commit 683731b into Instagram:main Sep 16, 2021
@rodrigozhou rodrigozhou deleted the fix_type_infer_provider branch September 16, 2021 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants