D8282: tests: consistently put #testcases at beginning of file

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D8282#123677 , @martinvonz 
wrote:
  
  > In D8282#123675 , @marmoute 
wrote:
  >
  >> Can you drop your change to `tests/test-push-race.t` from this patch ?
  >
  > That change is consistent with the other changes in putting `#requires` and 
`#testcases` first. I'll update the commit message.
  
  This change break the formatting of an otherwhise well formatted file 
(`tests/test-narrow-sparse.t`). Can you please drop it from this diff and move 
forward with the others?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In D8282#123675 , @marmoute 
wrote:
  
  > Can you drop your change to `tests/test-push-race.t` from this patch ?
  
  That change is consistent with the other changes in putting `#requires` and 
`#testcases` first. I'll update the commit message.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  Can you drop your change to `tests/test-push-race.t` from this patch ?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In D8282#123665 , @marmoute 
wrote:
  
  > That's a good point, doing it for `#require` too would be more consistent.
  > I would make a difference between test that do not make too much of an 
effort to have a title + early documentation and the one who actually make 
effort to have a formal format for their title and documentation (the one with 
`=\ntitle\n=` or `title\n=`).
  > I would be fine with having the requires/testcases right after the title 
when it make senses. I think there are some instance where the variants are 
formally documented, and the `#testcase` block is part of that. It might make 
sense to keep them at that location.
  
  I'll leave this patch as is and leave the moving of comments above 
`#testcase` and `#requires` for you to do in a follow-up (if you care enough 
about it; I still prefer having the all these `#` statements at the top).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  That's a good point, doing it for `#require` too would be more consistent.
  
  I would make a difference between test that do not make too much of an effort 
to have a title + early documentation and the one who actually make effort to 
have a formal format for their title and documentation (the one with 
`=\ntitle\n=` or `title\n=`).
  
  I would be fine with having the requires/testcases right after the title when 
it make senses. I think there are some instance where the variants are formally 
documented, and the `#testcase` block is part of that. It might make sense to 
keep them at that location.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In D8282#123663 , @marmoute 
wrote:
  
  > In D8282#123661 , @martinvonz 
wrote:
  >
  >> (like imports and includes in other languages)
  >
  > Precisely, import and includes usually comes **after** the initial module 
licence, title and documentation. So I would like the testcase declaration to 
comes after the initial module title (and maybe doc).
  
  That's fair. I assume we should do the same with `#require` in that case? 
That'll be quite a bit larger, I think (because I think we usually put them 
before the test description).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D8282#123661 , @martinvonz 
wrote:
  
  > (like imports and includes in other languages)
  
  Precisely, import and includes usually comes **after** the initial module 
licence, title and documentation. So I would like the testcase declaration to 
comes after the initial module title (and maybe doc).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  I very much agree with Matt
  
  In D8282#123660 , @marmoute 
wrote:
  
  > I would rather have the large title (===\ntitle\n===) be the very first 
things in the test for clarify.
  
  I'm like @mharbison72 and mostly skip past the #testcases and #requires at 
the top (like imports and includes in other languages), so they don't bother me 
at all. Sounds like we have two votes in favor and one against. Let's see what 
others feel.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  I would rather have the large title (===\ntitle\n===) be the very first 
things in the test for clarify.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[mharbison72 (Matt Harbison)]

mharbison72 added inline comments.
mharbison72 accepted this revision.

INLINE COMMENTS

> martinvonz wrote in test-push-race.t:1-3
> > Please avoir putting content before the
> > test file title.
> 
> Why? (I personally don't find this hard to read.)

I'd go further and say it's easier to read when `#testcases` is the very first 
line, as it doesn't get visually swallowed by nesting between two long lines.  
(I didn't realize they didn't have to be the first line, and wouldn't have 
looked anywhere else either.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute, mharbison72
Cc: mharbison72, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[martinvonz (Martin von Zweigbergk)]

martinvonz added inline comments.

INLINE COMMENTS

> marmoute wrote in test-push-race.t:1-3
> Please avoir putting content before the test file title.
> (and maybe keep the documentation closer to them.)

> Please avoir putting content before the
> test file title.

Why? (I personally don't find this hard to read.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  The other change looks good and I would take them if they were not in the 
same changeset as the change in tests/test-push-race.t

INLINE COMMENTS

> test-push-race.t:1-3
> +#testcases strict unrelated
> +
>  
> 

Please avoir putting content before the test file title.
(and maybe keep the documentation closer to them.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8282/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

To: martinvonz, durin42, #hg-reviewers
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8282: tests: consistently put #testcases at beginning of file

[martinvonz (Martin von Zweigbergk)]

martinvonz created this revision.
Herald added a reviewer: durin42.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I think it's misleading to put `#testcases` statements further down in
  the file because any statements before it will be executed in all
  cases. I also find them easier to find at the beginning of the file.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8282

AFFECTED FILES
  tests/test-fix-topology.t
  tests/test-narrow-sparse.t
  tests/test-push-race.t

CHANGE DETAILS

diff --git a/tests/test-push-race.t b/tests/test-push-race.t
--- a/tests/test-push-race.t
+++ b/tests/test-push-race.t
@@ -1,3 +1,5 @@
+#testcases strict unrelated
+
 

 Test cases where there are race condition between two clients pushing to the 
same repository
 

@@ -7,6 +9,10 @@
 perform its push. The "raced" client starts its actual push after the "racing"
 client push is fully complete.
 
+We tests multiple cases:
+* strict: no race detected,
+* unrelated: race on unrelated heads are allowed.
+
 A set of extension and shell functions ensures this scheduling.
 
   $ cat >> delaypush.py << EOF
@@ -113,12 +119,6 @@
   > graph = log -G --rev 'sort(all(), "topo")'
   > EOF
 
-We tests multiple cases:
-* strict: no race detected,
-* unrelated: race on unrelated heads are allowed.
-
-#testcases strict unrelated
-
 #if strict
 
   $ cat >> $HGRCPATH << EOF
diff --git a/tests/test-narrow-sparse.t b/tests/test-narrow-sparse.t
--- a/tests/test-narrow-sparse.t
+++ b/tests/test-narrow-sparse.t
@@ -1,7 +1,8 @@
+#testcases tree flat
+
 Testing interaction of sparse and narrow when both are enabled on the client
 side and we do a non-ellipsis clone
 
-#testcases tree flat
   $ . "$TESTDIR/narrow-library.sh"
   $ cat << EOF >> $HGRCPATH
   > [extensions]
diff --git a/tests/test-fix-topology.t b/tests/test-fix-topology.t
--- a/tests/test-fix-topology.t
+++ b/tests/test-fix-topology.t
@@ -1,3 +1,5 @@
+#testcases obsstore-off obsstore-on
+
 A script that implements uppercasing all letters in a file.
 
   $ UPPERCASEPY="$TESTTMP/uppercase.py"
@@ -30,7 +32,6 @@
 we'll test it with evolution off and on. This only changes the revision
 numbers, if all is well.
 
-#testcases obsstore-off obsstore-on
 #if obsstore-on
   $ cat >> $HGRCPATH < [experimental]



To: martinvonz, durin42, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel