Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork61
Cherry-picker: Remove BACKPORT_COMPLETE. UNSET state after finished.#315
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Mariatta commentedMar 15, 2019
webknjaz commentedMar 15, 2019
Checking it now. |
webknjaz commentedMar 15, 2019
@Mariatta I expected that state would be reset if everything is completed successfully. How about calling |
webknjaz commentedMar 15, 2019
Any non-allowed states are supposed idicate that something went wrong. |
Mariatta commentedMar 15, 2019
Ok I've removed BACKPORT_COMPLETE. unset will be called instead, and using the UNSET state. |
Mariatta commentedMar 15, 2019
uugh don't know what's the deal with travis 😟 Tests-passing-locally-for-me™️ |
webknjaz commentedMar 15, 2019
Oh, that's weird... Did you run all tests locally or just related? |
Mariatta commentedMar 15, 2019
Hmm I ran everything |
webknjaz commentedMar 15, 2019
I checked this branch locally as well and it passes. |
webknjaz commentedMar 15, 2019
@Mariatta just to be on the safe side, could you please clear the cache in Travis CI? It's herehttps://travis-ci.org/python/core-workflow/caches |
Mariatta commentedMar 15, 2019
I've cleared cache and restart the build, still no luck 🤕 Anyone else can try this locally? |
🐍🌚🤖@Mariatta, I've formatted these files using
|
webknjaz commentedMar 15, 2019
webknjaz commentedMar 15, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@Mariatta I know what the problem is! 🎉 So |
webknjaz commentedMar 15, 2019
Solutions:
- &install-and-test-cherry-picker...git:depth:200... |
🐍🌚🤖@Mariatta, I've formatted these files using
|
Uh oh!
There was an error while loading.Please reload this page.
cherry_picker/cherry_picker/test.py Outdated
| ], | ||
| ) | ||
| @mock.patch("os.path.exists") | ||
| @mock.patch("cherry_picker.cherry_picker.validate_sha") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
These tests aren't testing validate_sha itself, but the other functionalities. I think it is ok to be mocked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
webknjaz left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think that mockingvalidate_sha is wrong. This way you'll just completely disable testing of the specific code path. It doesn't cure the root of the issue but only symptoms. Along with that, it makes the project vulnerable to regressions.
Mariatta commentedMar 17, 2019
hm, but changing it to recent sha also only delays the problem, same issue will happen again in the future. |
🐍🌚🤖@Mariatta, I've formatted these files using
|
webknjaz commentedMar 17, 2019
@Mariatta I think you should just make git clone deeper. Or if you're worried about the delay, it's possible to disable |
🐍🌚🤖@Mariatta, I've formatted these files using
|
Mariatta commentedMar 17, 2019
Thanks, I've disabled git depth in travis CI and reverted the mocks. |
webknjaz commentedMar 17, 2019
@Mariatta great! P.S. One small feedback about |
Mariatta commentedMar 17, 2019
Thanks! |

No description provided.