Page MenuHomePhabricator

tests: Adapt expected output for minor differences with rhg
ClosedPublic

Authored by SimonSapin on Mar 9 2021, 4:40 AM.

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

SimonSapin created this revision.Mar 9 2021, 4:40 AM
baymax updated this revision to Diff 26194.Mar 9 2021, 8:40 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

marmoute requested changes to this revision.Mar 9 2021, 1:16 PM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
tests/test-basic.t
10 ↗(On Diff #26194)

You can use (rhg !) for this.

Alternatively, we could also set it since if does not use normal hg invocation.

tests/test-globalopts.t
267–271

Why do we need a fallback here ?

tests/test-hgrc.t
62

What's the possible difference here ?

tests/test-ssh.t
406–423

I don't think the behavior in the alternative block is correct. It look like the think above is trying to flag some potential security issue. I recommend we don't record the behavior of rhg not doing it and instead keep the if with a comment to flag this for future investigation. (This is fine if rhg does not handling this yet)

This revision now requires changes to proceed.Mar 9 2021, 1:16 PM
Alphare added inline comments.
tests/test-hgrc.t
62
SimonSapin added inline comments.Mar 11 2021, 4:42 AM
tests/test-basic.t
10 ↗(On Diff #26194)

I’ve seen other tests do something similar, but it’s not documented in https://www.mercurial-scm.org/wiki/WritingTests#Filtering_output and I filed to find the code implementing it in run-tests.py so I didn’t know how to use it. Does it mark a line that is there when that feature is enabled or disabled? Is ! a negation?

I considered always setting this but the current run-tests.py code doesn’t have a meaningful value when in non-rhg mode.

tests/test-globalopts.t
267–271

Whatever error causes a Python traceback (either inexistent cwd or config parse error I suppose?) goes through Rust code instead when in rhg mode, so there is no traceback. chg already had the same behavior so that seemed reasonable.

tests/test-hgrc.t
62

Right, the Display impl for io::Error prints the numeric errno value next to the error string: https://github.com/rust-lang/rust/blob/1.50.0/library/std/src/io/error.rs#L538-L541 so the output is (Permission denied (os error 13))

tests/test-ssh.t
406–423

Yes, abort: potentially unsafe serve indeed blocks a security issue in the serve command. As far as I can tell the current rhg behavior is safe since it aborts (although with a different message) before getting anywhere near the serve.

Keeping the if but removing the else simply reduces test coverage. Having a test expect the current behavior even if it’s not the "ideal" behavior seems preferable to me as it lets us find out if a patch changes it accidentally.

marmoute added inline comments.Mar 12 2021, 11:44 AM
tests/test-ssh.t
406–423

Should we use (missing-correct-output !) (known-bad-output !) there then ?

SimonSapin updated this revision to Diff 26296.Mar 12 2021, 5:07 PM
marmoute accepted this revision.Mar 15 2021, 5:42 AM
baymax updated this revision to Diff 26341.Mar 15 2021, 9:10 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.

I am like to all time see this get help file explorer windows 10 file and function