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.

Sign up for GitHub

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

Jump to bottom

Port test_cpp_models.py to pytest #4039

Closed
wants to merge 1 commit into from
Closed

Port test_cpp_models.py to pytest #4039

wants to merge 1 commit into from

Conversation

AnirudhDagar
Copy link
Contributor

Port test_cpp_models.py to pytest.

@NicolasHug
Copy link
Member

Thanks for the PR @AnirudhDagar !

Following up on #3992, I think we should only port these tests if we can actually fix them. Otherwise, we'll keep forgetting about them and they'll stay broken forever.

So I'd suggest to either:

  • try to fix the current failures - when I tried locally, only some of the mnasnet tests were failing, so hopefully this should be reasonably simple. If you manage to fix them we can probably heavily factorize the tests into a single one, with some parameatrization.
  • drop the PR if you don't feel like fixing the tests, which is totally OK!!

@AnirudhDagar
Copy link
Contributor Author

Thanks @NicolasHug, I'll close the PR for now and if/when I'm able to fix these I'll open it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AnirudhDagar @NicolasHug @facebook-github-bot