Re: [PATCH] tests: fix autostash

2013-06-08 Thread Felipe Contreras
On Fri, Jun 7, 2013 at 10:29 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index a5e69f3..ff370a3 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -71,8 +71,7 @@

Re: [PATCH] tests: fix autostash

2013-06-08 Thread Ramkumar Ramachandra
Felipe Contreras wrote: Yes you do. The rest of the tests expect that the previous rebase has been aborted. In fact, all the tests depend on the previous test finishing correctly, which is not the way tests should be written. How else am I supposed to write them? If there is a stale state

Re: [PATCH] tests: fix autostash

2013-06-08 Thread Felipe Contreras
On Sat, Jun 8, 2013 at 8:19 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: Doing 'rm -rf $dotest' is even worst than 'git rebase --abort', because it relies on the implementation of 'git rebase', which might need to remove more files than $dotest. Huh? Tests

Re: [PATCH] tests: fix autostash

2013-06-08 Thread Ramkumar Ramachandra
Felipe Contreras wrote: Ofcourse they're implementation details. Even in this very test, I check $dotest/autostash plenty of times. The more the test relies on implementation details, the worst. I'm not convinced about this. Then show me how to do it correctly. Something like this.

Re: [PATCH] tests: fix autostash

2013-06-08 Thread Felipe Contreras
On Sat, Jun 8, 2013 at 9:04 AM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: Ofcourse they're implementation details. Even in this very test, I check $dotest/autostash plenty of times. The more the test relies on implementation details, the worst. I'm not

Re: [PATCH] tests: fix autostash

2013-06-08 Thread Antoine Pelisse
On Sat, Jun 8, 2013 at 4:04 PM, Ramkumar Ramachandra artag...@gmail.com wrote: Felipe Contreras wrote: Ofcourse they're implementation details. Even in this very test, I check $dotest/autostash plenty of times. The more the test relies on implementation details, the worst. I'm not

Re: [PATCH] tests: fix autostash

2013-06-08 Thread Ramkumar Ramachandra
Antoine Pelisse wrote: The more the test relies on implementation details, the worst. I'm not convinced about this. My understanding of these tests is that they make sure new/better implementations don't break the user experience/defined behavior. If the test relies on the implementation,

Re: [PATCH] tests: fix autostash

2013-06-08 Thread Jonathan Nieder
Ramkumar Ramachandra wrote: How else am I supposed to write them? If there is a stale state from the previous test, there isn't too much I can do. Or should I be cleaning up state at the beginning of each test, instead of at the end? That's one strategy. test_when_finished to restore the

[PATCH] tests: fix autostash

2013-06-07 Thread Felipe Contreras
We should call 'git rebase --abort', like a normal user would do. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- t/t3420-rebase-autostash.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index

Re: [PATCH] tests: fix autostash

2013-06-07 Thread Ramkumar Ramachandra
Felipe Contreras wrote: diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index a5e69f3..ff370a3 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -71,8 +71,7 @@ testrebase() { test_must_fail git rebase$type related-onto-branch