Merge lp:~spiv/bzr-usertest/exit_codes into lp:~bzr/bzr-usertest/trunk-old

[Andrew Bennetts]

[Andrew Bennetts (spiv)]

Fixes exit_codes handling so that specifying exit_codes={...} in compile_* functions works properly. Adds some tests, too.

This allows the 'network' suite to pass with --strict.

[Andrew Bennetts (spiv)]

Btw, in the long run I think probably it would be more elegant to encode the exit code in the script strings, rather than needing to provide a label and then a dict that uses that label. e.g.:

    self.compile_default("$tool foo ## label [42]")

But this patch at least fixes the mechanism that was already there.

[Martin Pool (mbp)]

Putting this in a pseudo-comment seems a inconsistent with the general shell-like syntax used within usertest. If it was executed by an actual shell then we might want something like

  expectrc 1 bzr diff

where

  expectrc() {
    rc=$1
    shift
    "$@"
    [ $? -ne $rc ] || echo 'fail'
  }

Of course it's not actually run through the shell.

I see the concept of having labels in ## comments is already established in usertest so fair enough then.

+ def test_exit_codes(self):
+ task = userscript.ScriptTask()
+ task.compile_default("""
+ $python -c 'raise SystemExit(42)' ## mylabel
+ """, exit_codes={'mylabel': 42})
+ # No exception is raised, because the exit code matches the expected
+ # exit code given to compile_default.
+ (measurements,created_dirs) = task.run_script_for(usertool.TOOL_BZR,
+ {'python': sys.executable}, [], strict=True)

On Windows these would need to be doublequotes not single. I'm not sure how close the test suite is to working on Windows so it may not matter. At any rate it's easy to change later.

I would have expected to see a test for the case of a command that succeeds when it's expected to fail. It looks like that will pass though.

review: Approve

[Ian Clatworthy (ian-clatworthy)]

I like this feature - thanks for adding it.

I'd like to see the label you're using changed from 'bzr-diff-changed' to just 'diff' or 'diff-changed' though, because the script applies to multiple tools (self.compile_default), not just bzr.

review: Needs Fixing