[Updated] D10135: tests: Adapt expected output for minor differences with rhg

SimonSapin phabricator at mercurial-scm.org
Thu Mar 11 09:42:09 UTC 2021


SimonSapin added inline comments.

INLINE COMMENTS

> marmoute wrote in test-basic.t:10
> You can use ` (rhg !)` for this.
> 
> Alternatively, we could also set it since if does not use normal `hg` invocation.

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.

> marmoute wrote in test-globalopts.t:267-271
> Why do we need a fallback here ?

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.

> Alphare wrote in test-hgrc.t:62
> Maybe it's the same as this issue I had a while ago: https://www.mercurial-scm.org/repo/hg/file/74e2256a56b8/tests/common-pattern.py#l126

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))`

> marmoute wrote in 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)

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.

REPOSITORY
  rHG Mercurial

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

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

To: SimonSapin, #hg-reviewers, marmoute
Cc: Alphare, marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210311/f04a2193/attachment-0002.html>


More information about the Mercurial-patches mailing list