search

PyUtilib / pyutilib

A collection of general Python utilities, including logging and file IO, subprocess management, plugin systems, and workflow management.
BSD 3-Clause "New" or "Revised" License
34 stars 20 forks source link

Fix COLUMN width in pyutilib.misc config tests #69

Closed whart222 closed 4 years ago

whart222 commented 4 years ago

  1. Hard-coding the test wrapping length, to ensure that tests pass consistently.

  2. Removing print() statements in tests, to make them less verbose.

Fixes: N/A

Summary/Motivation:

This resolves new test failures that have arisen when using Python 3.8

Changes proposed in this PR:

Hard-coding configuration of pyutilib.misc tests.

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.
coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 61.039% when pulling 31a3fd3b5e5e26b3cbf495e56d6ec7b4d0fd2568 on fix_config_tests into 6591015234b400865573eebcb710e534e60ce13d on master.

jsiirola commented 4 years ago

What is the motivation for this PR? More specifically, what is the purpose of setting the COLUMNS environment variable or setting the (apparently unused) self.maxDiff = None?

I am alo not entirely clear as to the motivation for removing the print statements: that output should be suppressed by nosetests when tests pass. However, when tests fail, the raw printed output is far easier to compare with the baseline than the diff produced by assert equal.

whart222 commented 4 years ago

These tests failed using Anaconda Python 3.8 on a new machine, because the word-wrap length differed in that context from previous/other contexts. This change merely hard-codes the word-wrap length to ensure that tests will always pass.

whart222 commented 4 years ago

In my opinion, debugging print() statements should not be included in tests simply to support debugging of tests. These can easily be added when tests fail. Even though nose suppresses this output, such output does slow down the testing. And as we've seen with large test suites in other projects, the time needed to run tests can become a bottleneck.

whart222 commented 4 years ago

@jsiirola Is there a reason to delay approval of this PR further? Did this just get lost in the shuffle?

jsiirola commented 4 years ago

This just got lost... I had a minor change to ensure that tearDown doesn't remove COLUMNS from the environment if it was already there. Once tests pass, we should merge.