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

Proposed by Andrew Bennetts
Status: Merged
Merged at revision: not available
Proposed branch: lp:~spiv/bzr-usertest/exit_codes
Merge into: lp:~bzr/bzr-usertest/trunk-old
To merge this branch: bzr merge lp:~spiv/bzr-usertest/exit_codes
Reviewer Review Type Date Requested Status
Ian Clatworthy Needs Fixing
Martin Pool (community) Approve
Review via email: mp+1309@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote :

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.

Revision history for this message
Andrew Bennetts (spiv) wrote :

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.

Revision history for this message
Martin Pool (mbp) wrote :

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
Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

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

Subscribers

People subscribed via source and target branches

to all changes: