napalm-automation / napalm-base

Apache License 2.0
33 stars 48 forks source link

Convert napalm_base testing to tox #284

Closed bewing closed 6 years ago

bewing commented 7 years ago

Convert napalm_base to tox testing.

py27 tests clean, I don't have py34 installed locally, and the new mock driver fails tests on 3.5 and 3.6.

Will dig into tomorrow.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling d90ec6795ff6ff579e82192bb204e1e19cd31069 on bewing:tox into on napalm-automation:develop.

bewing commented 7 years ago

@ktbyers @dbarrosop If you have the spare time to investigate the py3 errors in test_mock_driver, I'd appreciate it, otherwise I'll try to get to it this week/weekend

ogenstad commented 7 years ago

Try changing this in test_basic() in test/unit/test_mock_driver.py

From:

        with pytest.raises(napalm_base.exceptions.ConnectionClosedException) as excinfo:
            d.get_facts()
        assert "connection closed" in excinfo.value

To:

        with pytest.raises(napalm_base.exceptions.ConnectionClosedException) as excinfo:
            d.get_facts()
        assert "connection closed" in str(excinfo.value)

Looks like it could be the same for the other errors.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 744c1df4d84b4c61ef9d1a2b38ddf4553c80e1f5 on bewing:tox into on napalm-automation:develop.

coveralls commented 6 years ago

Coverage Status

Changes Unknown when pulling 81673c96986789ff3ab303ce577d3f51b9175959 on bewing:tox into on napalm-automation:develop.

bewing commented 6 years ago

Should we write tests for napalm_base/tests/*, or just omit them from the coverage report?

ktbyers commented 6 years ago

@bewing I think we should omit them from coverage report.

coveralls commented 6 years ago

Coverage Status

Changes Unknown when pulling 60212887f5fca2566c99f0808f9f556e84fcae91 on bewing:tox into on napalm-automation:develop.

dbarrosop commented 6 years ago

It's nice to see the last bump to 72% :P

dbarrosop commented 6 years ago

LGTM, @ktbyers any objections?

coveralls commented 6 years ago

Coverage Status

Changes Unknown when pulling d59fbd41b3931e350cb7adaedd99d2cf2213b9cb on bewing:tox into on napalm-automation:develop.

dbarrosop commented 6 years ago

Let's merge it! :D