Abstract away git repo during tests by ZainRizvi · Pull Request #82594 · pytorch/pytorch

ZainRizvi · 2022-08-01T19:35:44Z

Description

During unit tests we want to return fake data for any methods that rely on a local git repository. Otherwise those tests will fail when run in an environment that doesn't have the pytorch repo checked out (such as fbinternal or a tarball download)

Also refactored existing tests to use the new DummyGitRepo

Issue

N/A

Testing

Validating old behavior:
Removed the .git folder locally to effectively delete the git repository. Then ran python3 .github/scripts/test_trymerge.py to ensure the test failed

Validating fix:
Ran python3 .github/scripts/test_trymerge.py both with and without the .git folder

pytorch-bot · 2022-08-01T19:35:47Z

🔗 Helpful Links

🧪 See artifacts and rendered test results here

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures, 3 Pending

As of commit 295387a:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

zengk95

lgtm pending lint

huydhn

LGTM! Just FYI that the OLD test before this PR could also fail when you do a shallow checkout, i.e. git clone --depth 1, which is used commonly in CI. It's still a git repo, but it wouldn't have the commit you are looking for :P

malfet

Please fix lint, otherwise LGTM
Consider nit

.github/scripts/test_trymerge.py

Co-authored-by: Nikita Shulga <nshulga@fb.com>

ZainRizvi · 2022-08-01T21:35:46Z

@pytorchmergebot merge

pytorchmergebot · 2022-08-01T21:37:26Z

@pytorchbot successfully started a merge and created land time checks. See merge status here and land check progress here

pytorchmergebot · 2022-08-01T21:42:30Z

Merge failed due to Refusing to merge as mandatory check(s) Lint failed for rule OSS CI
Raised by https://github.com/pytorch/pytorch/actions/runs/2778033540

ZainRizvi · 2022-08-01T22:01:52Z

@pytorchmergebot merge

pytorchmergebot · 2022-08-01T22:03:19Z

@pytorchbot successfully started a merge and created land time checks. See merge status here and land check progress here

### Description During unit tests we want to return fake data for any methods that rely on a local git repository. Otherwise those tests will fail when run in an environment that doesn't have the pytorch repo checked out (such as fbinternal or a tarball download) Also refactored existing tests to use the new DummyGitRepo ### Issue N/A ### Testing Validating old behavior: Removed the `.git` folder locally to effectively delete the git repository. Then ran `python3 .github/scripts/test_trymerge.py` to ensure the test failed Validating fix: Ran `python3 .github/scripts/test_trymerge.py` both with and without the `.git` folder Pull Request resolved: #82594 Approved by: https://github.com/zengk95, https://github.com/huydhn, https://github.com/malfet

pytorchmergebot · 2022-08-02T00:58:58Z

Merge failed due to Failed to merge; some land checks failed: pull, pull / linux-docs / build-docs (cpp)
Raised by https://github.com/pytorch/pytorch/actions/runs/2778162819

kit1980 · 2022-08-02T03:59:17Z

@pytorchmergebot merge

pytorchmergebot · 2022-08-02T04:00:36Z

@pytorchbot successfully started a merge and created land time checks. See merge status here and land check progress here

### Description During unit tests we want to return fake data for any methods that rely on a local git repository. Otherwise those tests will fail when run in an environment that doesn't have the pytorch repo checked out (such as fbinternal or a tarball download) Also refactored existing tests to use the new DummyGitRepo ### Issue N/A ### Testing Validating old behavior: Removed the `.git` folder locally to effectively delete the git repository. Then ran `python3 .github/scripts/test_trymerge.py` to ensure the test failed Validating fix: Ran `python3 .github/scripts/test_trymerge.py` both with and without the `.git` folder Pull Request resolved: #82594 Approved by: https://github.com/zengk95, https://github.com/huydhn, https://github.com/malfet

github-actions · 2022-08-02T06:57:21Z

Hey @ZainRizvi.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Summary: ### Description During unit tests we want to return fake data for any methods that rely on a local git repository. Otherwise those tests will fail when run in an environment that doesn't have the pytorch repo checked out (such as fbinternal or a tarball download) Also refactored existing tests to use the new DummyGitRepo ### Issue N/A ### Testing Validating old behavior: Removed the `.git` folder locally to effectively delete the git repository. Then ran `python3 .github/scripts/test_trymerge.py` to ensure the test failed Validating fix: Ran `python3 .github/scripts/test_trymerge.py` both with and without the `.git` folder Pull Request resolved: #82594 Approved by: https://github.com/zengk95, https://github.com/huydhn, https://github.com/malfet Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/87a11f112b1d7a172fb98004421dc98a35d25415 Reviewed By: kit1980 Differential Revision: D38359475 Pulled By: ZainRizvi fbshipit-source-id: 21c58d074468162e0de5d5701250d098e89f1f91

Abstract away git repo during tests

5cc1467

facebook-github-bot added the cla signed label Aug 1, 2022

ZainRizvi marked this pull request as ready for review August 1, 2022 19:36

ZainRizvi requested a review from a team as a code owner August 1, 2022 19:36

ZainRizvi requested a review from malfet August 1, 2022 19:36

zengk95 approved these changes Aug 1, 2022

View reviewed changes

huydhn approved these changes Aug 1, 2022

View reviewed changes

malfet approved these changes Aug 1, 2022

View reviewed changes

.github/scripts/test_trymerge.py Outdated Show resolved Hide resolved

.github/scripts/test_trymerge.py Outdated Show resolved Hide resolved

kit1980 mentioned this pull request Aug 1, 2022

[pytorch][mergebot][EASY] Fix test case for trymerge #82608

Closed

ZainRizvi and others added 2 commits August 1, 2022 16:26

fix linter error

444aefa

remove YAGNI

35ca9c0

Co-authored-by: Nikita Shulga <nshulga@fb.com>

more linter fixes

295387a

pytorchmergebot added the Merged label Aug 2, 2022

pytorchmergebot closed this in 87a11f1 Aug 2, 2022

ZainRizvi deleted the zainr/fix-tests branch September 30, 2022 22:25

Conversation

ZainRizvi commented Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue

Testing

Uh oh!

pytorch-bot bot commented Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results here

✅ No Failures, 3 Pending

Uh oh!

zengk95 left a comment

Choose a reason for hiding this comment

Uh oh!

huydhn left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

malfet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ZainRizvi commented Aug 1, 2022

Uh oh!

pytorchmergebot commented Aug 1, 2022

Uh oh!

pytorchmergebot commented Aug 1, 2022

Uh oh!

ZainRizvi commented Aug 1, 2022

Uh oh!

pytorchmergebot commented Aug 1, 2022

Uh oh!

pytorchmergebot commented Aug 2, 2022

Uh oh!

kit1980 commented Aug 2, 2022

Uh oh!

pytorchmergebot commented Aug 2, 2022

Uh oh!

github-actions bot commented Aug 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ZainRizvi commented Aug 1, 2022 •

edited

Loading

pytorch-bot bot commented Aug 1, 2022 •

edited

Loading

huydhn left a comment •

edited

Loading