D5981: tests: drop a few unnecessary "(glob)"

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

D5981: tests: drop a few unnecessary "(glob)"

pulkit (Pulkit Goyal)
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I believe thise are unnecessary since https://phab.mercurial-scm.org/rHGa8d3a4be066e0d3b3d01a90a98dc1907cf4fad3b (windows: use
  util.localpath for repo-relative paths in getuipathfn(), 2019-02-11),
  but someone with a Windows machine should confirm.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  tests/test-status-color.t
  tests/test-status.t

CHANGE DETAILS

diff --git a/tests/test-status.t b/tests/test-status.t
--- a/tests/test-status.t
+++ b/tests/test-status.t
@@ -116,11 +116,11 @@
   ? ../b/in_b
   ? ../in_root
   $ HGPLAIN=1 hg status --cwd a --config ui.tweakdefaults=yes
-  ? a/1/in_a_1 (glob)
-  ? a/in_a (glob)
-  ? b/1/in_b_1 (glob)
-  ? b/2/in_b_2 (glob)
-  ? b/in_b (glob)
+  ? a/1/in_a_1
+  ? a/in_a
+  ? b/1/in_b_1
+  ? b/2/in_b_2
+  ? b/in_b
   ? in_root
   $ HGPLAINEXCEPT=tweakdefaults hg status --cwd a --config ui.tweakdefaults=yes
   ? 1/in_a_1
@@ -163,11 +163,11 @@
   ? ../b/in_b
   ? ../in_root
   $ HGPLAIN=1 hg status --cwd a
-  ? a/1/in_a_1 (glob)
-  ? a/in_a (glob)
-  ? b/1/in_b_1 (glob)
-  ? b/2/in_b_2 (glob)
-  ? b/in_b (glob)
+  ? a/1/in_a_1
+  ? a/in_a
+  ? b/1/in_b_1
+  ? b/2/in_b_2
+  ? b/in_b
   ? in_root
 
 if relative paths are explicitly off, tweakdefaults doesn't change it
diff --git a/tests/test-status-color.t b/tests/test-status-color.t
--- a/tests/test-status-color.t
+++ b/tests/test-status-color.t
@@ -31,11 +31,11 @@
   [status.unknown|? ][status.unknown|in_root]
 HGPLAIN disables color
   $ HGPLAIN=1 hg status --color=debug
-  ? a/1/in_a_1 (glob)
-  ? a/in_a (glob)
-  ? b/1/in_b_1 (glob)
-  ? b/2/in_b_2 (glob)
-  ? b/in_b (glob)
+  ? a/1/in_a_1
+  ? a/in_a
+  ? b/1/in_b_1
+  ? b/2/in_b_2
+  ? b/in_b
   ? in_root
 HGPLAINEXCEPT=color does not disable color
   $ HGPLAINEXCEPT=color hg status --color=debug



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5981: tests: drop a few unnecessary "(glob)"

pulkit (Pulkit Goyal)
mharbison72 added a comment.


  The tests run clean with this.
 
  But I'm a bit confused.  The test harness has been doing '\' -> '/' conversion without (glob) now for a little over a year, and it complains if there's a trailing (glob) and no '\' -> '/' conversion.  That's not been happening here, and made me suspicious.  These globs predate that functionality slightly, so I'm not sure the meaning of the referenced commit (which seems to say nothing will change because ui.slash is set).
 
  To be clear, these paths are still being printed with '\', and the test harness is accommodating that.  But I'm not sure if that's what you intended.

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers, mharbison72
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5981: tests: drop a few unnecessary "(glob)"

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D5981#87341, @mharbison72 wrote:
 
  > The tests run clean with this.
  >
  > But I'm a bit confused.  The test harness has been doing '\' -> '/' conversion without (glob) now for a little over a year, and it complains if there's a trailing (glob) and no '\' -> '/' conversion.  That's not been happening here, and made me suspicious.  These globs predate that functionality slightly, so I'm not sure the meaning of the referenced commit (which seems to say nothing will change because ui.slash is set).
 
 
  You're right, and even if I go back to https://phab.mercurial-scm.org/rHGbdcaf612e75a08f20257076845510353a448b57d (where you added these globs), it seems like it should have been converted to slashes already there (because the test runner set ui.slash back then, too, and the code seemed to respect that). Do you have time to go back and see if the globbing was never needed?
 
  Maybe the "no complaining from test harness" is because these lines contain a "?", which is a glob character, so maybe it doesn't warn then?
 
  > To be clear, these paths are still being printed with '\', and the test harness is accommodating that.  But I'm not sure if that's what you intended.
 
  Yes, I hope that my recent changes has made it so more commands, not fewer, print paths in the native form.

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers, mharbison72
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D5981: tests: drop a few unnecessary "(glob)"

pulkit (Pulkit Goyal)
In reply to this post by pulkit (Pulkit Goyal)
mharbison72 added a comment.


  In https://phab.mercurial-scm.org/D5981#87343, @martinvonz wrote:
 
  > In https://phab.mercurial-scm.org/D5981#87341, @mharbison72 wrote:
  >
  > > The tests run clean with this.
  > >
  > > But I'm a bit confused.  The test harness has been doing '\' -> '/' conversion without (glob) now for a little over a year, and it complains if there's a trailing (glob) and no '\' -> '/' conversion.  That's not been happening here, and made me suspicious.  These globs predate that functionality slightly, so I'm not sure the meaning of the referenced commit (which seems to say nothing will change because ui.slash is set).
  >
  >
  > You're right, and even if I go back to https://phab.mercurial-scm.org/rHGbdcaf612e75a08f20257076845510353a448b57d (where you added these globs), it seems like it should have been converted to slashes already there (because the test runner set ui.slash back then, too, and the code seemed to respect that). Do you have time to go back and see if the globbing was never needed?
 
 
  It was needed.  In fact the test runner itself added it on Windows.  (There's logic in there to add it to the output if changing '\' to '/' results in a match.)  I bisected the `HGPLAIN=1 hg status --cwd a` test back to https://phab.mercurial-scm.org/rHG7e3b145f824793ccdb5caf4f13570d4f25ab0164, where the trail went cold- it fails there too where it was added.  Backing up one more and reverting the test to that also fails, so it wasn't a code change there causing it.
 
  (As an aside, it would be awesome if bisect accepted a --force to ignore the dirty working copy check.  Then I could take out the globs, and see where it started working without having to manually update to each candidate.  I guess the question then is whether to merge the dirty changes on each update, or just revert to the dirty changes as-is.  I guess I'd lean towards the latter for automated bisection.)
 
  > Maybe the "no complaining from test harness" is because these lines contain a "?", which is a glob character, so maybe it doesn't warn then?
 
  Strangely, no.  I added a (glob) to a random status test that listed only the file with an unknown status in https://phab.mercurial-scm.org/rHG37a0ad669051ab70b89aaf669a7c277a908cf4b4 and https://phab.mercurial-scm.org/rHG37b33c34bf4f890857b5e8728febbc82a99368a5, and they both took away the stray (glob).
 
  >> To be clear, these paths are still being printed with '\', and the test harness is accommodating that.  But I'm not sure if that's what you intended.
  >
  > Yes, I hope that my recent changes has made it so more commands, not fewer, print paths in the native form.

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers, mharbison72
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel