Re: [PATCH 3/7] add a test for semantic errors in config files
On 7/24/2014 3:41 AM, Junio C Hamano wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'check line errors for malformed values' ' + cp .git/config .git/config.old Should this be mv not cp? You will be overwriting the file from scratch in the later part of this test. Noted. + test_when_finished mv .git/config.old .git/config + echo [alias]\n br .git/config Is the use of \n portable? Yes, you are right, will replace with printf in the next patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] add a test for semantic errors in config files
Tanay Abhra tanay...@gmail.com writes: + test_when_finished mv .git/config.old .git/config + echo [alias]\n br .git/config Is the use of \n portable? Yes, you are right, will replace with printf in the next patch. ... or a cat .git/config \EOF, since this is the construct used elsewhere in the script. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] add a test for semantic errors in config files
Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: + test_when_finished mv .git/config.old .git/config + echo [alias]\n br .git/config Is the use of \n portable? Yes, you are right, will replace with printf in the next patch. ... or a cat .git/config \EOF, since this is the construct used elsewhere in the script. That sounds much better ;-) Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] add a test for semantic errors in config files
Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'check line errors for malformed values' ' + cp .git/config .git/config.old + test_when_finished mv .git/config.old .git/config + echo [alias]\n br .git/config + test_expect_code 128 git br 2result + grep fatal: bad config file line 2 in .git/config result +' + The test fails at this point in history. I guess the problem will disapear once you've put PATCH 2 at the right point in the series. Another option is to mark the test as test_expect_failure when you introduce it, and change it to test_expect_success when you fix it (probably not applicable here, but it's a trick I find elegant). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] add a test for semantic errors in config files
Matthieu Moy matthieu@grenoble-inp.fr writes: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'check line errors for malformed values' ' +cp .git/config .git/config.old Should this be mv not cp? You will be overwriting the file from scratch in the later part of this test. +test_when_finished mv .git/config.old .git/config +echo [alias]\n br .git/config Is the use of \n portable? Another option is to mark the test as test_expect_failure when you introduce it, and change it to test_expect_success when you fix it (probably not applicable here, but it's a trick I find elegant). Yes, I agree that it is a good practice to document an existing breakage in an early patch #1, and then make a fix and flip expect-failure to expect-success in the patch #2. Breaking the code and documenting the breakage by expecting a failure in one patch, and then later fixing the breakage and flipping the expectation in another patch, is a bit less nice, though ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
