Skip to content

Conversation

casteryh
Copy link
Contributor

@casteryh casteryh commented Sep 5, 2025

No description provided.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 5, 2025
@casteryh casteryh marked this pull request as ready for review September 5, 2025 16:59
Copilot

This comment was marked as outdated.

@allenwang28
Copy link
Contributor

Maybe we can use pytest.importorskip instead? from https://stackoverflow.com/a/42520506

@casteryh
Copy link
Contributor Author

casteryh commented Sep 5, 2025

Maybe we can use pytest.importorskip instead? from https://stackoverflow.com/a/42520506

I think we should explicitly mark these tests as not tested by CI and try to fix CI and migrate these no-CI tests after, instead of burying it.

Adding an automatic pytest skip further complicates it as the test may or may not have been run, and the only way to know is to dig in the logs. For example, if we do introduce a genuine import error in the future, then whole test will be skipped (even locally) , which we probably don't want.

@allenwang28
Copy link
Contributor

sure we can explicitly mark these tests as skipped. IMO the right way to do this is with pytest with like pytest.mark.skip or something rather than having a separate directory for more tests

@casteryh casteryh changed the title move the test that doesn't run on CI to a new folder Skip tests when there are import issues on CI worker Sep 8, 2025
@casteryh
Copy link
Contributor Author

casteryh commented Sep 8, 2025

sure we can explicitly mark these tests as skipped. IMO the right way to do this is with pytest with like pytest.mark.skip or something rather than having a separate directory for more tests

If you say so. Done.

Copy link
Contributor

@allenwang28 allenwang28 left a comment

Choose a reason for hiding this comment

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

Thanks @casteryh, we should remove this once the pip install paths are working!

@allenwang28 allenwang28 merged commit a3d2466 into meta-pytorch:main Sep 9, 2025
5 checks passed
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 Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants