Page MenuHomePhabricator

test-dirstate-race: hide irrelevant hg status output
ClosedPublic

Authored by sid0 on Jul 19 2017, 11:33 PM.

Details

Summary

See the explanation for more.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sid0 created this revision.Jul 19 2017, 11:33 PM
sid0 added a comment.Jul 19 2017, 11:38 PM

(I think this should go into stable as a "test fix")

martinvonz added inline comments.
tests/test-dirstate-race.t
226–227

The commit that introduced this test case (15e85dded933) referred to bug https://bz.mercurial-scm.org/show_bug.cgi?id=5581. The description of that issue (filed by you) finishes with "That last 'hg status' should be empty, but it returns 'M b'.". You changed your mind about what the expected behavior should be?

sid0 added inline comments.Jul 20 2017, 12:42 AM
tests/test-dirstate-race.t
226–227

The *last* hg status should be empty. This is an intermediate hg status.

sid0 added inline comments.Jul 20 2017, 12:43 AM
tests/test-dirstate-race.t
236

^^ this one should be empty.

martinvonz added inline comments.Jul 20 2017, 1:24 AM
tests/test-dirstate-race.t
226–227

Oh, the "hg status" is just to cause it to write the dirstate? Would "hg debugrebuilddirstate" work? That doesn't produce any output and it's clearer that that will result in the dirstate getting written ("hg status" doesn't always write it, as I'm sure you know). If that would work, we can delete the whole comment, I think.

sid0 added inline comments.Jul 20 2017, 12:04 PM
tests/test-dirstate-race.t
226–227

I'm afraid not -- debugrebuilddirstate will try to acquire the wlock, which is already held by the rebase.

martinvonz added inline comments.Jul 20 2017, 12:33 PM
tests/test-dirstate-race.t
226–227

Makes sense (for others' info: "hg status" will write the dirstate if the lock can be acquired, but will not wait for the lock, IIRC).

How about piping the result of that "hg status" to /dev/null and adding a comment above it saying that we're only calling it to cause the dirstate to be written if the repo is not locked? Then you can get rid of the comment above.

sid0 updated this revision to Diff 351.Jul 20 2017, 9:04 PM
sid0 retitled this revision from test-dirstate-race: replace BROKEN line with explanation of changed output to test-dirstate-race: hide irrelevant hg status output.Jul 20 2017, 9:05 PM
martinvonz accepted this revision.Jul 21 2017, 12:49 AM
This revision is now accepted and ready to land.Jul 21 2017, 12:49 AM
This revision was automatically updated to reflect the committed changes.