Merge lp://staging/~sseman/juju-ci-tools/model-change-watcher-py3-2 into lp://staging/juju-ci-tools

Proposed by Seman
Status: Needs review
Proposed branch: lp://staging/~sseman/juju-ci-tools/model-change-watcher-py3-2
Merge into: lp://staging/juju-ci-tools
Prerequisite: lp://staging/~sseman/juju-ci-tools/model-change-watcher-py3
Diff against target: 288 lines (+244/-11)
3 files modified
Makefile (+6/-3)
jujupy.py (+0/-8)
tests_py3/test_assess_model_change_watcher.py (+238/-0)
To merge this branch: bzr merge lp://staging/~sseman/juju-ci-tools/model-change-watcher-py3-2
Reviewer Review Type Date Requested Status
Nicholas Skaggs (community) Approve
Curtis Hovey (community) code Needs Information
Review via email: mp+312755@code.staging.launchpad.net

Description of the change

This is the second branch that adds unit tests for the assess_model_change_watcher.py script.

It also updates the Makefile so that the right version of Python is used to run unittest and flake8. "unittest" doesn't make it simple to exclude some files, example excluding Python 3 files when running Python 2 tests, therefore I created a different directory for Python 3 test scripts.

First branch:
https://code.launchpad.net/~sseman/juju-ci-tools/model-change-watcher-py3/+merge/312523

To post a comment you must log in.
1796. By Seman

Removed event.

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

I'll wait until the pre-req is landed. However, 1 comment below.

Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you. I think the Makefile changes are incomplete.

review: Needs Information (code)
Revision history for this message
Seman (sseman) wrote :

I don't know if it works with all OSes you mentioned but I do see your point.

Instead of breaking "make test", I can add a different rule for Python 3 unit tests "test_python3" and only run tests under "tests_py3" directory.

Instead of using the find, grep, xargs commands for lint test, I will exclude the files explicitly and add a different rule for Python 3.
 py3="assess_model_change_watcher.py test_assess_model_change_watcher.py"
 lint:
    flake8 $$(find -name '*.py') --builtins xrange,basestring --exclude $(py3)
 lint_python3:
    flake8 $(py3)

Does the above seem a reasonable solution?

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Thank you Seman.

review: Approve

Unmerged revisions

1796. By Seman

Removed event.

1795. By Seman

Updated Makefile.

1794. By Seman

Added unit tests.

1793. By Seman

Merged.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches