| Message ID | 20190221214059.9195-2-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase: fix 2.11.0-era --fork-point regression | expand |
On Thu, Feb 21, 2019 at 10:40:58PM +0100, Ævar Arnfjörð Bjarmason wrote: > Add tests rebasing a linear branch topology to linear rebase tests > added in 2aad7cace2 ("add simple tests of consistency across rebase > types", 2013-06-06). I had trouble parsing this. Did you mean s/topology to/topology, similar to/? > These tests are duplicates of two surrounding tests that do the same > with tags pointing to the same objects. Right now there's no change in > behavior being introduced, but as we'll see in a subsequent change > rebase can have different behaviors when working implicitly with > remote tracking branches. It took me a while to figure out what was new in these tests. Maybe: These tests are duplicates of two surrounding tests, but with one change: the existing tests refer to objects by their tag names, but here we'll use branches (pointing at the same objects). But then I'm left wondering why that's important. > While I'm at it add a --fork-point test, strictly speaking this is > redundant to the existing '' test, as no argument to rebase implies > --fork-point. But now it's easier to grep for tests that explicitly > stress --fork-point. That makes sense. > +test_expect_success 'setup branches and remote tracking' ' > + git tag -l >tags && > + for tag in $(cat tags) > + do > + git branch branch-$tag $tag || return 1 > + done && I don't think we need this extra tmpfile and cat, do we? I.e., for tag in $(git tag -l) would work. We should probably avoid depending on the exact output of the porcelain "tag", though. Maybe: git for-each-ref \ --format='create refs/heads/branch-%(refname:strip=2) %(objectname)' \ refs/tags | git update-ref --stdin which has the added bonus of using a constant number of processes. -Peff
Jeff King <peff@peff.net> writes: >> While I'm at it add a --fork-point test, strictly speaking this is >> redundant to the existing '' test, as no argument to rebase implies >> --fork-point. But now it's easier to grep for tests that explicitly >> stress --fork-point. > > That makes sense. > >> +test_expect_success 'setup branches and remote tracking' ' >> + git tag -l >tags && >> + for tag in $(cat tags) >> + do >> + git branch branch-$tag $tag || return 1 >> + done && > > I don't think we need this extra tmpfile and cat, do we? I.e., > > for tag in $(git tag -l) > > would work. I think it is being (overly) defensive not to lose the exit status of "git tag". > We should probably avoid depending on the exact output of > the porcelain "tag", though. Maybe: > > git for-each-ref \ > --format='create refs/heads/branch-%(refname:strip=2) %(objectname)' \ > refs/tags | > git update-ref --stdin > > which has the added bonus of using a constant number of processes. Much better ;-)
diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index 7274dca40b..b847064f91 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -31,6 +31,16 @@ test_run_rebase success -m test_run_rebase success -i test_have_prereq !REBASE_P || test_run_rebase success -p +test_expect_success 'setup branches and remote tracking' ' + git tag -l >tags && + for tag in $(cat tags) + do + git branch branch-$tag $tag || return 1 + done && + git remote add origin "file://$PWD" && + git fetch origin +' + test_run_rebase () { result=$1 shift @@ -57,10 +67,28 @@ test_run_rebase () { " } test_run_rebase success '' +test_run_rebase success --fork-point test_run_rebase success -m test_run_rebase success -i test_have_prereq !REBASE_P || test_run_rebase failure -p +test_run_rebase () { + result=$1 + shift + test_expect_$result "rebase $* -f rewrites even if remote upstream is an ancestor" " + reset_rebase && + git rebase $* -f branch-b branch-e && + ! test_cmp_rev branch-e origin/branch-e && + test_cmp_rev branch-b HEAD~2 && + test_linear_range 'd e' branch-b.. + " +} +test_run_rebase success '' +test_run_rebase success --fork-point +test_run_rebase success -m +test_run_rebase success -i +test_have_prereq !REBASE_P || test_run_rebase success -p + test_run_rebase () { result=$1 shift @@ -71,6 +99,7 @@ test_run_rebase () { " } test_run_rebase success '' +test_run_rebase success --fork-point test_run_rebase success -m test_run_rebase success -i test_have_prereq !REBASE_P || test_run_rebase success -p
Add tests rebasing a linear branch topology to linear rebase tests added in 2aad7cace2 ("add simple tests of consistency across rebase types", 2013-06-06). These tests are duplicates of two surrounding tests that do the same with tags pointing to the same objects. Right now there's no change in behavior being introduced, but as we'll see in a subsequent change rebase can have different behaviors when working implicitly with remote tracking branches. While I'm at it add a --fork-point test, strictly speaking this is redundant to the existing '' test, as no argument to rebase implies --fork-point. But now it's easier to grep for tests that explicitly stress --fork-point. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t3421-rebase-topology-linear.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)