-
Notifications
You must be signed in to change notification settings - Fork 10
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add checkout branch tests from the official suite #193
Conversation
cmd/branch.go
Outdated
if err != nil { | ||
return err | ||
} | ||
err = branch.DeleteBranch(c) |
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.
how come this one's not using the more Go-ish if err := branch.DeleteBranch(c); err { [..] }
? (Same with a bunch of err =
checks below below)
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.
Thanks, I'll clean this up shortly.
@@ -23,4 +23,4 @@ script: | |||
# t1004.17 needs "git merge-recursive" (which also isn't documented) | |||
# t1014.26 needs "git config --unset-all" | |||
# t3000.7 requires git pack-refs | |||
- GIT_SKIP_TESTS='t0008.30[5-9] t0008.310 t0008.321 t0008.323 t0008.37[0-9] t0008.38[0-7] t0008.39[1-2] t1004.1[6-7] t1014.26 t3000.7' ./official-git/run-tests.sh t0000-basic.sh t0004-unwritable.sh t0007-git-var.sh t0008-ignores.sh t0010-racy-git.sh t0062-revision-walking.sh t0070-fundamental.sh t0081-line-buffer.sh t1000-read-tree-m-3way.sh t1001-read-tree-m-2way.sh t1002-read-tree-m-u-2way.sh t1003-read-tree-prefix.sh t1004-read-tree-m-u-wf.sh t1005-read-tree-reset.sh t1008-read-tree-overlay.sh t1009-read-tree-new-index.sh t1011-read-tree-sparse-checkout.sh t1012-read-tree-df.sh t1014-read-tree-confusing.sh t1100-commit-tree-options.sh t1304-default-acl.sh t1403-show-ref.sh t2000-checkout-cache-clash.sh t2001-checkout-cache-clash.sh t2002-checkout-cache-u.sh t2006-checkout-index-basic.sh t2009-checkout-statinfo.sh t2014-switch.sh t2100-update-cache-badpath.sh t3000-ls-files-others.sh t3002-ls-files-dashpath.sh t3006-ls-files-long.sh t3020-ls-files-error-unmatch.sh t3800-mktag.sh t4113-apply-ending.sh t4123-apply-shrink.sh t7062-wtstatus-ignorecase.sh t7511-status-index.sh | |||
- GIT_SKIP_TESTS='t0008.30[5-9] t0008.310 t0008.321 t0008.323 t0008.37[0-9] t0008.38[0-7] t0008.39[1-2] t1004.1[6-7] t1014.26 t3000.7 t2018.[6-9] t2018.1[5-8]' ./official-git/run-tests.sh t0000-basic.sh t0004-unwritable.sh t0007-git-var.sh t0008-ignores.sh t0010-racy-git.sh t0062-revision-walking.sh t0070-fundamental.sh t0081-line-buffer.sh t1000-read-tree-m-3way.sh t1001-read-tree-m-2way.sh t1002-read-tree-m-u-2way.sh t1003-read-tree-prefix.sh t1004-read-tree-m-u-wf.sh t1005-read-tree-reset.sh t1008-read-tree-overlay.sh t1009-read-tree-new-index.sh t1011-read-tree-sparse-checkout.sh t1012-read-tree-df.sh t1014-read-tree-confusing.sh t1100-commit-tree-options.sh t1304-default-acl.sh t1403-show-ref.sh t2000-checkout-cache-clash.sh t2001-checkout-cache-clash.sh t2002-checkout-cache-u.sh t2006-checkout-index-basic.sh t2009-checkout-statinfo.sh t2014-switch.sh t2018-checkout-branch.sh t2100-update-cache-badpath.sh t3000-ls-files-others.sh t3002-ls-files-dashpath.sh t3006-ls-files-long.sh t3020-ls-files-error-unmatch.sh t3800-mktag.sh t4113-apply-ending.sh t4123-apply-shrink.sh t7062-wtstatus-ignorecase.sh t7511-status-index.sh |
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.
What are t2018.[6-9] t2018.1[5-8] testing for?
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.
Some of those tests are checking for whether there is handling for "mergeable changes." There's a couple that are failing for reasons I haven't investigated yet because they work when I try them manually. It's probably some kind of setup or verification problem. I hope to have a look at them soon. One of them is using a rev notation that dgit probably doesn't support yet.
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.
yeah, it's usually something like that (or an error caused by one of those things in an earlier test cascading down to later ones), I was just curious what they were in this case since they're new ignores being added.
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.
Unfortunately, the official git tests make it kind of hard to see the exact commands and error message or even assertion failures in a failing test. It's sometimes a bit of a guessing game which dgit command is misbehaving.
Yes, I don't like adding ignores right off the bat but the benefit is some extra test coverage of the checkout command.
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'm pretty sure you already know this, but using the parameters -v -d -i
with the official test suite makes it easier.. (one of them prints what it's doing, one of them quits at the first failure, and one of them leaves the temp directory behind after exiting so you can inspect it..) From there you can work backwards by manually injecting exit 1
in the test and inspecting the directory until you figure out where the first place it doesn't look like you expect (or looks different than what the official git client does when it runs the test suite.)
In the process of looking at implementing #105 it appeared that the support is already there. This PR brings in the official checkout tests to verify the functionality moving forward. In order to support the tests branch move and delete were added.