Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from nose to unittest. #1434

Merged
merged 10 commits into from
Dec 23, 2019
Merged

Switch from nose to unittest. #1434

merged 10 commits into from
Dec 23, 2019

Conversation

waylan
Copy link
Member

@waylan waylan commented Mar 13, 2018

The nose docs state:

Nose has been in maintenance mode for the past several years and will
likely cease without a new person/team to take over maintainership.
New projects should consider using Nose2, py.test, or just plain
unittest/unittest2.

As we aren't really using any of nose's features, its easiest to switch
to the standard lib unittest.

Note This is currently experimental. I'm checking to see how many tests fail and if there are differences across platforms. Is it more work than its worth?

@waylan
Copy link
Member Author

waylan commented Mar 13, 2018

Hmm, interesting that the "unittests" tests only fail with Python<3.4 (2.7, 3.3, & pypy2 only). And the minimum requirements tests seem to have some anomaly with the Markdown TOC extension that needs investigating. I find it curious that these tests all pass with nose (the test count matches so no test discovery issues). What is nose doing differently and could these failures occur in the real world? In other words, is nose hiding some problems?

@waylan
Copy link
Member Author

waylan commented Mar 16, 2018

So all of the failing tests rely on __file__ when constructing paths. According to the Python 3.4 release notes:

Module __file__ attributes (and related values) should now always contain absolute paths by default, with the sole exception of __main__.__file__ when a script has been executed directly using a relative path. (Contributed by Brett Cannon in issue 18416.)

The problem is that when __file__ returns a relative path (as it does in Python < 3.4), os.path.abspath(__file__) is essentially the same as os.path.join(os.getcwd(), __file__) which could easily be wrong. In f4856cb I got one failing test to pass, but that's only because I have MkDocs installed with setuptool's develop mode and the CWD is the correct directory by coincidence when running the test locally. Based on Travis' output, it looks like the same coincidence occurs there. But the whole thing feels very fragile.

Curious why these errors don't occur under nose. Is nose somehow overriding the behavior of __file__ for Python < 3.4?

In any event, I'm inclined to leave this for now. We can revisit the issue when one of the following happens:

  1. We drop support for all versions of Python < 3.4.
  2. The existing tests get rewritten to no longer rely on __file__.

@waylan
Copy link
Member Author

waylan commented Apr 12, 2018

A test refactor might also benefit from using the testfixtures lib. A bunch of helpers in that lib would simplify a bunch of our tests.

@waylan
Copy link
Member Author

waylan commented Jun 29, 2018

In #1504 we introduce a tempdir decorator to use for testing (which was inspired in part by the testfitures lib mentioned in the previous comment). It makes it relatively simple to setup and tear down temp dirs and files on a test-by-test basis. Perhaps we should use temp files rather than having tests refer to the integration test files to avoid the inconsistent behavior of __file__. Unfortunately, that would require a lot of work as quite a few tests rely in the integration test files.

@waylan waylan force-pushed the no-nose branch 2 times, most recently from 36943a9 to 5c30c30 Compare September 5, 2018 17:53
@waylan
Copy link
Member Author

waylan commented Sep 5, 2018

This has been rebased against master. Many of the same problems exist, although some have been removed and others added. we are still extensively relying on __file__ in the tests.

The nose docs state:

> Nose has been in maintenance mode for the past several years and will
> likely cease without a new person/team to take over maintainership.
> New projects should consider using Nose2, py.test, or just plain
> unittest/unittest2.

As we aren't really using any of nose's features, its easiest to switch
to the standard lib unittest.
@waylan
Copy link
Member Author

waylan commented Dec 23, 2019

Now that we are PY35+ only (see #1936 & #1938), I rebased this and resolved merge conflicts.

Interestingly, all tests pass without issue. However, we have a bunch of deprecation warnings (and the like) in the test output which nose apparently was hiding (see here for example). I guess it makes sense to clean them up as part of our dropping-PY27 process.

@waylan
Copy link
Member Author

waylan commented Dec 23, 2019

I cleaned up all warnings except 2 as those are directly related to which version of Markdown is installed and would require changing the minimum version supported to (3.0). We'll handle that separately.

@waylan waylan merged commit 399f842 into mkdocs:master Dec 23, 2019
@waylan waylan deleted the no-nose branch December 23, 2019 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant