Skip to content
This repository was archived by the owner on Jun 26, 2026. It is now read-only.

Integration test for node false negative pattern #895

Open
esohel30 wants to merge 4 commits into
facebook:mainfrom
esohel30:integration_test
Open

Integration test for node false negative pattern #895
esohel30 wants to merge 4 commits into
facebook:mainfrom
esohel30:integration_test

Conversation

@esohel30

Copy link
Copy Markdown
Contributor

adding integration test for node false negative pattern.

@tianhan0 tianhan0 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for adding the test!

Could you also include the expected results for this test (e.g., the *.models files)? Otherwise the CI tests would fail.
You can run Pysa on all integration tests by executing OUNIT_SHARDS=1 dune exec interprocedural_analyses/taint/test/integrationTest.exe under pyre/source. This would

  1. either create new expected results for new tests (such as this PR), because there is no existing expected results.
  2. or create the actual results, when they differ from the existing expected results.

Additionally, could we

  1. rename this test to be more descriptive, such as heap_alias.py? We try to let the name of each integration test be as descriptive as possible.
  2. add a few comments in the .py file to explain why this is a false negative. You could paraphrase Maxime's explanation (in the chat).

@kinto0

kinto0 commented Aug 14, 2024

Copy link
Copy Markdown
Contributor

cc @tianhan0 for updated review

@kinto0
kinto0 requested a review from tianhan0 August 14, 2024 18:11
@tianhan0

Copy link
Copy Markdown

cc @tianhan0 for updated review

@kinto0 I don't think this PR has been updated in the past month. But we will keep an eye on this -- if the author (who is an MLH fellow) does not update this in another month or so, we will abandon this PR and recreate it internally.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants