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

Fix/doc tests #35

Merged
merged 26 commits into from
Mar 4, 2017
Merged

Fix/doc tests #35

merged 26 commits into from
Mar 4, 2017

Conversation

neomonkeus
Copy link
Member

@neomonkeus neomonkeus commented Feb 27, 2017

@niftools/pyffi-reviewers

Overview

Incremental increases in test suite execution working towards all tests being executed.
Consolidation of test structure to support above.

Fixes Known Issues

N/A

Documentation

Holding off on docs until we take a decision if we can co-execute automation tests that run pytest via Travis/Appvoyer and local tests executing via doc tests.

Testing

Expanded number of tests suite being executed.
Performed maintenance updates on doctests.
Structural improvements, i.e moved files around the place to mirror prod structure better.

Manual

All manual tests mainly relate to tests run via doctests.

  • Majority of doctest tests fixed, not really the goal of the PR but nice to get some progress.
  • These are not included in automated tests suite, so coverage won't reflect accurately doctest/currently manual related changes.

So...bonus coverage awaits if we can get these included in automation.
The reason not included as part of this PR is :

  • improved pass rate of about 30+ test cases working again, mainly low hanging fruit, not all tests are passing. Further effort required to identify if test or code issue. Main goal of PR is get framework up and running. Separate issues will be written when we get down individual tests failure point.
  • Need to decide effort required unify running of all tests via pytests or provide an alternative method of execution for travis/appvoyer or convert to another system.

They aren't currently run via the pytest automation as they would break due to relative path issues.
At a later stage, attempt to figure if it is worth unifying how they run together through local execution, relative.

Automated

  • Increased test coverage by 10% -> 36%

Additional Information

The process of introducing more tests will be incremental, but I think there is enough content.
These maybe replaced at a later stage as paths are not very well supported.
This does not contribute to current coverage for automation.

Known issue :
reading tests/nif/test_opt_dupgeomdata.nif
    + Warning: read failed due corrupt file, corrupt format description, or bug.
Depending on where it is run, it could be within the module so would return __init__
or could be run a package level where it would have full pyffi.format.cfg path.
Updated to use recursive directory lookup because windows python decides to do things differently
@neomonkeus neomonkeus added this to the Version 2.2.3 milestone Feb 27, 2017
@neomonkeus neomonkeus self-assigned this Feb 27, 2017
@coveralls
Copy link

coveralls commented Feb 27, 2017

Coverage Status

Coverage increased (+5.8%) to 34.364% when pulling 977d1d8 on neomonkeus:fix/doc_tests into de11acd on niftools:develop.

Copy link
Member

@psi29a psi29a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few questions, the rest looks OK to me.

# 'tests/nif/opt_vertex_cache.txt',

# 'tests/kfm/kfmtoaster.txt',
# 'docs-sphinx/intro.rst',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these to be added later?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, same issues as previous. These text files are a way to define doctests external to the source code. They also suffer from relative path issues if run from this context and also have general failures.

@@ -57,39 +59,46 @@
suite = unittest.TestSuite()
for mod in mods:
try:
suite.addTest(doctest.DocTestSuite(mod))
pass
#suite.addTest(doctest.DocTestSuite(mod))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be added later?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they are currently excluded as they will fail when not run from a local context

@neomonkeus
Copy link
Member Author

@psi29a are you happy with the review?

@psi29a
Copy link
Member

psi29a commented Mar 4, 2017

I am, we can build on this. :)

@neomonkeus neomonkeus merged commit a0060a8 into niftools:develop Mar 4, 2017
@neomonkeus neomonkeus deleted the fix/doc_tests branch March 24, 2017 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants