Skip to content

add CI job for prototype tests #4664

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
Merged
merged 6 commits into from
Oct 20, 2021
Merged

add CI job for prototype tests #4664

merged 6 commits into from
Oct 20, 2021

Conversation

pmeier
Copy link
Collaborator

@pmeier pmeier commented Oct 20, 2021 •
edited by pytorch-probot bot
Loading

@pmeier pmeier requested a review from fmassa October 20, 2021 09:17
datumbox
datumbox reviewed Oct 20, 2021
View reviewed changes
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. We are also very interested in adding tests on prototype models. See #4655.

I think we will adjust our approach based on your proposal.

cc @jdsgomes

@pmeier
Copy link
Collaborator Author

pmeier commented Oct 20, 2021

CI job is operational: https://app.circleci.com/pipelines/github/pytorch/vision/11516/workflows/d46b293b-a579-4d84-ac55-062bd4a815ab/jobs/894445

I'll fix the warnings in a follow-up PR.

@pmeier pmeier requested a review from datumbox October 20, 2021 09:47
datumbox
datumbox approved these changes Oct 20, 2021
View reviewed changes
Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

Nice, LGTM! We can merge after green CI.

pmeier
pmeier commented Oct 20, 2021
View reviewed changes
pytest.ini
Comment on lines +9 to +10
--ignore=test/test_datasets_download.py
--ignore-glob=test/test_prototype_*.py
Copy link
Collaborator Author

@pmeier pmeier Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Unless we invoke pytest specifically with test/test_prototype_*.py, the prototype tests will now be ignored for the normal unittests.

I've applied the same treatment to the dataset download tests. With this, invoking pytest locally is now exactly the same as in CI minus the coverage. LMK if I should split this in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good. Personally I'm happy to keep it here.

@datumbox datumbox merged commit 247374b into pytorch:main Oct 20, 2021
@github-actions
Copy link

Hey @datumbox!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

fmassa
fmassa reviewed Oct 20, 2021
View reviewed changes
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Thanks @pmeier !

@pmeier pmeier deleted the prototype-ci branch October 20, 2021 12:15
@pmeier pmeier mentioned this pull request Oct 20, 2021
Closed
facebook-github-bot pushed a commit that referenced this pull request Oct 25, 2021
@fmassa @datumbox @facebook-github-bot
[fbsync] add CI job for prototype tests (#4664)
31fa539 
Summary:
* add CI job for prototype tests

* register prototype job

* fix torchvision installation

* fix test invocation

* ignore prototype tests in normal unittests

Reviewed By: datumbox

Differential Revision: D31898205

fbshipit-source-id: 3fcde83a5f6dceaa21d2572fb21d78348a842b9b

Co-authored-by: Vasilis Vryniotis <[email protected]>
cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
@pmeier @datumbox @cyyever
add CI job for prototype tests (pytorch#4664)
675c32b 
* add CI job for prototype tests

* register prototype job

* fix torchvision installation

* fix test invocation

* ignore prototype tests in normal unittests

Co-authored-by: Vasilis Vryniotis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@datumbox datumbox datumbox approved these changes

@fmassa fmassa fmassa left review comments

Assignees
No one assigned
Labels
ciflow/default cla signed module: ci module: tests new feature prototype
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add CI uniquely for torchdata prototype and remove torchdata from most CIs
4 participants
@pmeier @datumbox @fmassa @facebook-github-bot