{"thread":{"id":"41697","subject":"[PATCH] clone tests: rename t57* => t56*","startedAt":"2016-03-15T21:25:50Z","lastAt":"2016-03-15T22:09:03Z","messageCount":4,"participants":["Stefan Beller","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"280843","messageId":"1458077150-15564-1-git-send-email-sbeller@google.com","threadId":"41697","inReplyTo":null,"subject":"[PATCH] clone tests: rename t57* => t56*","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-15T21:25:50Z","receivedAt":"2016-03-15T21:25:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When trying to find a good spot for testing clone with submodules, I\ngot confused where to add a new test file. There are both tests in t560*\nas well as t57* both testing the clone command. t/README claims the\nsecond digit is to indicate the command, which is inconsistent to the\ncurrent naming structure.\n\nRename all t57* tests to be in t56* to follow the pattern of the digits\nas laid out in t/README.\n\nIt would have been less work to rename t56* => t57* because there are less\nfiles, but the tests in t56* look more basic and I assumed the higher the\nlast digits the more complicated niche details are tested, so with the patch\nnow it looks more in order to me.\n\nSigned-off-by: Stefan Beller \n---\n\nIf there is a reason to have 2 separate spaces for clone testing,\nand I missed it, we should document that why it is important to keep\nthem separate.\n\nThanks,\nStefan\n\n t/{t5700-clone-reference.sh => t5604-clone-reference.sh} | 0\n t/{t5701-clone-local.sh => t5605-clone-local.sh} | 0\n t/{t5702-clone-options.sh => t5606-clone-options.sh} | 0\n t/{t5704-bundle.sh => t5607-clone-bundle.sh} | 0\n t/{t5705-clone-2gb.sh => t5608-clone-2gb.sh} | 0\n t/{t5706-clone-branch.sh => t5609-clone-branch.sh} | 0\n t/{t5707-clone-detached.sh => t5610-clone-detached.sh} | 0\n t/{t5708-clone-config.sh => t5611-clone-config.sh} | 0\n t/{t5709-clone-refspec.sh => t5612-clone-refspec.sh} | 0\n t/{t5710-info-alternate.sh => t5613-info-alternate.sh} | 0\n 10 files changed, 0 insertions(+), 0 deletions(-)\n rename t/{t5700-clone-reference.sh => t5604-clone-reference.sh} (100%)\n rename t/{t5701-clone-local.sh => t5605-clone-local.sh} (100%)\n rename t/{t5702-clone-options.sh => t5606-clone-options.sh} (100%)\n rename t/{t5704-bundle.sh => t5607-clone-bundle.sh} (100%)\n rename t/{t5705-clone-2gb.sh => t5608-clone-2gb.sh} (100%)\n rename t/{t5706-clone-branch.sh => t5609-clone-branch.sh} (100%)\n rename t/{t5707-clone-detached.sh => t5610-clone-detached.sh} (100%)\n rename t/{t5708-clone-config.sh => t5611-clone-config.sh} (100%)\n rename t/{t5709-clone-refspec.sh => t5612-clone-refspec.sh} (100%)\n rename t/{t5710-info-alternate.sh => t5613-info-alternate.sh} (100%)\n\ndiff --git a/t/t5700-clone-reference.sh b/t/t5604-clone-reference.sh\nsimilarity index 100%\nrename from t/t5700-clone-reference.sh\nrename to t/t5604-clone-reference.sh\ndiff --git a/t/t5701-clone-local.sh b/t/t5605-clone-local.sh\nsimilarity index 100%\nrename from t/t5701-clone-local.sh\nrename to t/t5605-clone-local.sh\ndiff --git a/t/t5702-clone-options.sh b/t/t5606-clone-options.sh\nsimilarity index 100%\nrename from t/t5702-clone-options.sh\nrename to t/t5606-clone-options.sh\ndiff --git a/t/t5704-bundle.sh b/t/t5607-clone-bundle.sh\nsimilarity index 100%\nrename from t/t5704-bundle.sh\nrename to t/t5607-clone-bundle.sh\ndiff --git a/t/t5705-clone-2gb.sh b/t/t5608-clone-2gb.sh\nsimilarity index 100%\nrename from t/t5705-clone-2gb.sh\nrename to t/t5608-clone-2gb.sh\ndiff --git a/t/t5706-clone-branch.sh b/t/t5609-clone-branch.sh\nsimilarity index 100%\nrename from t/t5706-clone-branch.sh\nrename to t/t5609-clone-branch.sh\ndiff --git a/t/t5707-clone-detached.sh b/t/t5610-clone-detached.sh\nsimilarity index 100%\nrename from t/t5707-clone-detached.sh\nrename to t/t5610-clone-detached.sh\ndiff --git a/t/t5708-clone-config.sh b/t/t5611-clone-config.sh\nsimilarity index 100%\nrename from t/t5708-clone-config.sh\nrename to t/t5611-clone-config.sh\ndiff --git a/t/t5709-clone-refspec.sh b/t/t5612-clone-refspec.sh\nsimilarity index 100%\nrename from t/t5709-clone-refspec.sh\nrename to t/t5612-clone-refspec.sh\ndiff --git a/t/t5710-info-alternate.sh b/t/t5613-info-alternate.sh\nsimilarity index 100%\nrename from t/t5710-info-alternate.sh\nrename to t/t5613-info-alternate.sh\n-- \n2.7.0.rc0.46.g8f16ed4.dirty\n"},{"id":"280848","messageId":"20160315215121.GA30011@sigill.intra.peff.net","threadId":"41697","inReplyTo":"1458077150-15564-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] clone tests: rename t57* => t56*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-15T21:51:22Z","receivedAt":"2016-03-15T21:51:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 15, 2016 at 02:25:50PM -0700, Stefan Beller wrote:\n\n> When trying to find a good spot for testing clone with submodules, I\n> got confused where to add a new test file. There are both tests in t560*\n> as well as t57* both testing the clone command. t/README claims the\n> second digit is to indicate the command, which is inconsistent to the\n> current naming structure.\n> \n> Rename all t57* tests to be in t56* to follow the pattern of the digits\n> as laid out in t/README.\n> \n> It would have been less work to rename t56* => t57* because there are less\n> files, but the tests in t56* look more basic and I assumed the higher the\n> last digits the more complicated niche details are tested, so with the patch\n> now it looks more in order to me.\n\nThis seems like a good change to me. There definitely is a general sense\nof \"more complex things should come later\" in the test suite[1], so\npreserving the existing ordering seems reasonable.\n\n> If there is a reason to have 2 separate spaces for clone testing,\n> and I missed it, we should document that why it is important to keep\n> them separate.\n\nIt looks like it was just carelessness from long ago. 5600 was added by\n5508a616 (Feb 2006), and then t5700 in cf9dc653 (May 2006). For the\nlater ones, everybody probably just found or the other and built on top\n(some of us even found both at various times; looks like I added t5708\nin 2011 and t5603 last year).\n\nI checked whether there were any stragglers in topics in flight with:\n\n git log --oneline --name-status --diff-filter=A origin..origin/pu t\n\nbut I think we are good.\n\n-Peff\n\n[1] I actually don't think the test ordering matters all that much. I\n guess if you run the tests directly from the Makefile, it will stop\n at the most basic test that fails.\n\n Personally, I run them in longest-to-shortest via \"prove\", which\n helps parallelism, and gives you the full list of failed tests.\n Which is often a good piece of knowledge by itself (is it just one\n test, or did I horribly break some fundamental part of git?). And\n then I pick one of the failures to study based on what seems simple\n to debug, and/or the area I've been making changes in.\n\n But I dunno. Maybe other people really do care about the order. It\n doesn't hurt to roughly follow the \"simplest comes first\" ordering.\n"},{"id":"280850","messageId":"CAGZ79kZWMhL5kTHU8gdPvm=VqDb58H0ZkNyop=c=P6bLLY1JLg@mail.gmail.com","threadId":"41697","inReplyTo":"20160315215121.GA30011@sigill.intra.peff.net","subject":"Re: [PATCH] clone tests: rename t57* => t56*","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-15T22:00:09Z","receivedAt":"2016-03-15T22:00:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 15, 2016 at 2:51 PM, Jeff King wrote:\n> On Tue, Mar 15, 2016 at 02:25:50PM -0700, Stefan Beller wrote:\n>\n>> When trying to find a good spot for testing clone with submodules, I\n>> got confused where to add a new test file. There are both tests in t560*\n>> as well as t57* both testing the clone command. t/README claims the\n>> second digit is to indicate the command, which is inconsistent to the\n>> current naming structure.\n>>\n>> Rename all t57* tests to be in t56* to follow the pattern of the digits\n>> as laid out in t/README.\n>>\n>> It would have been less work to rename t56* => t57* because there are less\n>> files, but the tests in t56* look more basic and I assumed the higher the\n>> last digits the more complicated niche details are tested, so with the patch\n>> now it looks more in order to me.\n>\n> This seems like a good change to me. There definitely is a general sense\n> of \"more complex things should come later\" in the test suite[1], so\n> preserving the existing ordering seems reasonable.\n>\n>> If there is a reason to have 2 separate spaces for clone testing,\n>> and I missed it, we should document that why it is important to keep\n>> them separate.\n>\n> It looks like it was just carelessness from long ago. 5600 was added by\n> 5508a616 (Feb 2006), and then t5700 in cf9dc653 (May 2006). For the\n> later ones, everybody probably just found or the other and built on top\n> (some of us even found both at various times; looks like I added t5708\n> in 2011 and t5603 last year).\n>\n> I checked whether there were any stragglers in topics in flight with:\n>\n> git log --oneline --name-status --diff-filter=A origin..origin/pu t\n>\n> but I think we are good.\n>\n> -Peff\n>\n> [1] I actually don't think the test ordering matters all that much. I\n> guess if you run the tests directly from the Makefile, it will stop\n> at the most basic test that fails.\n>\n> Personally, I run them in longest-to-shortest via \"prove\", which\n> helps parallelism, and gives you the full list of failed tests.\n> Which is often a good piece of knowledge by itself (is it just one\n> test, or did I horribly break some fundamental part of git?). And\n> then I pick one of the failures to study based on what seems simple\n> to debug, and/or the area I've been making changes in.\n>\n> But I dunno. Maybe other people really do care about the order. It\n> doesn't hurt to roughly follow the \"simplest comes first\" ordering.\n\nTalking about ordering, I have two use cases\n\n1) Before sending out patches: \"git rebase -i -x make -x 'make test' \"\n to catch myself for doing stupid things.\n\n2) When developing new code, I alternate between running an indivdual test\n \"cd t && GIT_TRACE=1 ./t7400... -d -i -x -v \" and running prove for all tests\n to have a good check if there are other niches I missed.\n\nSo I do not really have strong preference for the right order, I even\nthought about\nomitting the paragraph from the commit message and wanted to put it into\nthe notes below, but then I figured I want to record it as I was\nfrustrated about\nthe commit messages from 2006 as they don't answer simple questions like\n\"Why did you use a different second digit?\", so I figured if anyone digs up my\ncommit eventually I want to record as much of my current reasoning even\nif it is minor to help a future developer?\n"},{"id":"280852","messageId":"20160315220903.GA30488@sigill.intra.peff.net","threadId":"41697","inReplyTo":"CAGZ79kZWMhL5kTHU8gdPvm=VqDb58H0ZkNyop=c=P6bLLY1JLg@mail.gmail.com","subject":"Re: [PATCH] clone tests: rename t57* => t56*","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-15T22:09:03Z","receivedAt":"2016-03-15T22:09:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 15, 2016 at 03:00:09PM -0700, Stefan Beller wrote:\n\n> Talking about ordering, I have two use cases\n> \n> 1) Before sending out patches: \"git rebase -i -x make -x 'make test' \"\n> to catch myself for doing stupid things.\n> \n> 2) When developing new code, I alternate between running an indivdual test\n> \"cd t && GIT_TRACE=1 ./t7400... -d -i -x -v \" and running prove for all tests\n> to have a good check if there are other niches I missed.\n\nYep, that is roughly my workflow, too (with the occasional \"make test &&\nmake install\" thrown in). :)\n\n> So I do not really have strong preference for the right order, I even\n> thought about omitting the paragraph from the commit message and\n> wanted to put it into the notes below, but then I figured I want to\n> record it as I was frustrated about the commit messages from 2006 as\n> they don't answer simple questions like \"Why did you use a different\n> second digit?\", so I figured if anyone digs up my commit eventually I\n> want to record as much of my current reasoning even if it is minor to\n> help a future developer?\n\nI agree it is worth mentioning that you considered the order. I think\nwhat you wrote was fine, though I probably would have said something\nlike:\n\n Moving t57* to higher digits in t56* preserves the existing ordering.\n That may or may not matter to anyone, but it does not hurt to keep it.\n\n-Peff\n"}]}