Patchwork D8146: tests: stabilize test-rename-merge2.t on Windows

login
register
mail settings
Submitter phabricator
Date Feb. 24, 2020, 10:47 p.m.
Message ID <differential-rev-PHID-DREV-dp2kffvffjbigvzbq27t-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45307/
State Superseded
Headers show

Comments

phabricator - Feb. 24, 2020, 10:47 p.m.
mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I have no idea why, but this shifted in b4057d001760 <https://phab.mercurial-scm.org/rHGb4057d0017601357a59a18f2996719302ae51765>.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  tests/test-rename-merge2.t

CHANGE DETAILS




To: mharbison72, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 24, 2020, 11:37 p.m.
marmoute added a comment.
marmoute accepted this revision.


  Since the line does not occurs on linux, this probably got affected by some other change without being noticed.

REPOSITORY
  rHG Mercurial

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

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

To: mharbison72, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
phabricator - Feb. 24, 2020, 11:55 p.m.
mharbison72 added a comment.
mharbison72 added a subscriber: martinvonz.


  In D8146#121267 <https://phab.mercurial-scm.org/D8146#121267>, @marmoute wrote:
  
  > Since the line does not occurs on linux, this probably got affected by some other change without being noticed.
  
  Makes sense for why it was missed, but I bisected it back to the referenced commit.  I know @martinvonz made some other related changes prior to that commit that had similar test fallout on Windows, but it isn't obvious to me why the referenced commit would change the order of when the background thread spins up.

REPOSITORY
  rHG Mercurial

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

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

To: mharbison72, #hg-reviewers, marmoute
Cc: martinvonz, marmoute, mercurial-devel
phabricator - Feb. 25, 2020, 12:14 a.m.
marmoute added a comment.


  Since run-test has no idea where to put (?) line with the surounding change, they often end up preserved at start/end of the changeset section. This is probably just something like that.

REPOSITORY
  rHG Mercurial

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

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

To: mharbison72, #hg-reviewers, marmoute
Cc: martinvonz, marmoute, mercurial-devel
phabricator - Feb. 25, 2020, 5:21 a.m.
martinvonz added a comment.


  In D8146#121271 <https://phab.mercurial-scm.org/D8146#121271>, @mharbison72 wrote:
  
  > In D8146#121267 <https://phab.mercurial-scm.org/D8146#121267>, @marmoute wrote:
  >
  >> Since the line does not occurs on linux, this probably got affected by some other change without being noticed.
  >
  > Makes sense for why it was missed, but I bisected it back to the referenced commit.  I know @martinvonz made some other related changes prior to that commit that had similar test fallout on Windows, but it isn't obvious to me why the referenced commit would change the order of when the background thread spins up.
  
  Perhaps the line doesn't actually happen before that commit? You can try removing the `(?)` in the commit before my commit and see if that line is actually printed. I'm thinking that that commit pushed the number of files to close above some threshold that made it get printed. In that case, it was probably some of my earlier commits that made it no longer get printed and we just didn't notice since `(?)` means optional (no error if it doesn't get printed).

REPOSITORY
  rHG Mercurial

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

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

To: mharbison72, #hg-reviewers, marmoute
Cc: martinvonz, marmoute, mercurial-devel
phabricator - Feb. 25, 2020, 6:01 a.m.
mharbison72 added a comment.


  In D8146#121277 <https://phab.mercurial-scm.org/D8146#121277>, @martinvonz wrote:
  
  > In D8146#121271 <https://phab.mercurial-scm.org/D8146#121271>, @mharbison72 wrote:
  >
  >> In D8146#121267 <https://phab.mercurial-scm.org/D8146#121267>, @marmoute wrote:
  >>
  >>> Since the line does not occurs on linux, this probably got affected by some other change without being noticed.
  >>
  >> Makes sense for why it was missed, but I bisected it back to the referenced commit.  I know @martinvonz made some other related changes prior to that commit that had similar test fallout on Windows, but it isn't obvious to me why the referenced commit would change the order of when the background thread spins up.
  >
  > Perhaps the line doesn't actually happen before that commit? You can try removing the `(?)` in the commit before my commit and see if that line is actually printed. I'm thinking that that commit pushed the number of files to close above some threshold that made it get printed. In that case, it was probably some of my earlier commits that made it no longer get printed and we just didn't notice since `(?)` means optional (no error if it doesn't get printed).
  
  I can confirm that it was required in e1ecfc7c84be <https://phab.mercurial-scm.org/rHGe1ecfc7c84be922c28aedd80bc2ebcbe1d415e5d>, and reconfirmed that it is needed in the latest code by removing the line from both.  I'm not too worried about why it popped up- it just seemed odd that it didn't with the earlier followup patch I added to fix similar conditionalizing of this exact text elsewhere, and the commit where it started failing looked innocuous.

REPOSITORY
  rHG Mercurial

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

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

To: mharbison72, #hg-reviewers, marmoute
Cc: martinvonz, marmoute, mercurial-devel
phabricator - Feb. 25, 2020, 6:05 a.m.
martinvonz added a comment.


  In D8146#121279 <https://phab.mercurial-scm.org/D8146#121279>, @mharbison72 wrote:
  
  > In D8146#121277 <https://phab.mercurial-scm.org/D8146#121277>, @martinvonz wrote:
  >
  >> In D8146#121271 <https://phab.mercurial-scm.org/D8146#121271>, @mharbison72 wrote:
  >>
  >>> In D8146#121267 <https://phab.mercurial-scm.org/D8146#121267>, @marmoute wrote:
  >>>
  >>>> Since the line does not occurs on linux, this probably got affected by some other change without being noticed.
  >>>
  >>> Makes sense for why it was missed, but I bisected it back to the referenced commit.  I know @martinvonz made some other related changes prior to that commit that had similar test fallout on Windows, but it isn't obvious to me why the referenced commit would change the order of when the background thread spins up.
  >>
  >> Perhaps the line doesn't actually happen before that commit? You can try removing the `(?)` in the commit before my commit and see if that line is actually printed. I'm thinking that that commit pushed the number of files to close above some threshold that made it get printed. In that case, it was probably some of my earlier commits that made it no longer get printed and we just didn't notice since `(?)` means optional (no error if it doesn't get printed).
  >
  > I can confirm that it was required in e1ecfc7c84be <https://phab.mercurial-scm.org/rHGe1ecfc7c84be922c28aedd80bc2ebcbe1d415e5d>, and reconfirmed that it is needed in the latest code by removing the line from both.  I'm not too worried about why it popped up- it just seemed odd that it didn't with the earlier followup patch I added to fix similar conditionalizing of this exact text elsewhere, and the commit where it started failing looked innocuous.
  
  Yeah, I'm not worried about it either. Thanks for checking. (I wasn't holding off queuing this one because of that, I was/am just busy with other stuff.)

REPOSITORY
  rHG Mercurial

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

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

To: mharbison72, #hg-reviewers, marmoute
Cc: martinvonz, marmoute, mercurial-devel

Patch

diff --git a/tests/test-rename-merge2.t b/tests/test-rename-merge2.t
--- a/tests/test-rename-merge2.t
+++ b/tests/test-rename-merge2.t
@@ -722,8 +722,8 @@ 
    ancestor: 924404dff337, local: 0b76e65c8289+, remote: bdb19105162a
    preserving b for resolve of b
    preserving rev for resolve of rev
+  starting 4 threads for background file closing (?)
    b: both renamed from a -> m (premerge)
-  starting 4 threads for background file closing (?)
   picked tool '* ../merge' for b (binary False symlink False changedelete False) (glob)
   merging b
   my b@0b76e65c8289+ other b@bdb19105162a ancestor a@924404dff337