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

Update the test runner to use the updated code in astropy 1.3 #1983

Merged
merged 5 commits into from May 28, 2018

Conversation

Projects
4 participants
@Cadair
Copy link
Member

commented Feb 1, 2017

This modifies our internal test runner (the thing that provides sunpy.self_test and python setup.py test) to be much closer to astropy's implementation. The differences between us and astropy now are the replacement of the astropy remote_data argument with online and offline and the addition of the figure argument.

This means we have many more options to self_test and setup.py test such as checking for open file handles, running pep8 checks etc.

@Cadair Cadair added the Tests label Feb 1, 2017

@pep8speaks

This comment has been minimized.

Copy link

commented Feb 1, 2017

Hello @Cadair! Thanks for updating the PR.

Line 20:1: E303 too many blank lines (3)

Line 44:101: E501 line too long (130 > 100 characters)
Line 122:31: E128 continuation line under-indented for visual indent
Line 123:31: E128 continuation line under-indented for visual indent
Line 124:31: E128 continuation line under-indented for visual indent
Line 127:31: E128 continuation line under-indented for visual indent
Line 128:31: E128 continuation line under-indented for visual indent
Line 970:5: E122 continuation line missing indentation or outdented

Line 87:101: E501 line too long (144 > 100 characters)
Line 127:101: E501 line too long (107 > 100 characters)

Line 55:101: E501 line too long (110 > 100 characters)
Line 56:101: E501 line too long (111 > 100 characters)

Comment last updated on May 26, 2018 at 11:19 Hours UTC

@Cadair Cadair added [Review] [WIP] and removed [Review] labels Feb 1, 2017

@Cadair Cadair force-pushed the Cadair:test_runner branch from 321052b to 27c861c Feb 3, 2017

@Cadair

This comment has been minimized.

Copy link
Member Author

commented Feb 9, 2017

there is a lot more todo on this, for some reason the doctests are not being picked up by pytest. Also I am working on porting the extra plugins for pytest that astropy uses to provide things like open file checking into a form where we can use them (see astropy/astropy#5770). Oh and it just fails, so that needs fixing as well.

@Cadair

This comment has been minimized.

Copy link
Member Author

commented May 17, 2017

I will close this for now, there may be a couple of different ways of achieving it, like using the new code I wrote in Astropy to expose our current pytest UI.

@Cadair

This comment has been minimized.

Copy link
Member Author

commented May 10, 2018

This should now be resurrected as since this was closed we have adopted doctestplus and remote_data and all the astropy pytest plugins have been broken out into seperate packages, which should make this whole thing a lot easier.

I think the largest challenge here is now the setup command, but it should be doable.

@Cadair Cadair added this to High Priority Features in SunPy 1.0 May 10, 2018

@Cadair Cadair reopened this May 10, 2018

@Cadair Cadair force-pushed the Cadair:test_runner branch from 149575e to 2a00351 May 10, 2018

@Cadair Cadair removed the Needs Adoption label May 10, 2018

@Cadair Cadair added this to the 1.0 milestone May 10, 2018

from sunpy.tests import main as self_test
from sunpy.tests.runner import SunPyTestRunner

self_test = SunPyTestRunner.make_test_runner_in(__path__[0])

This comment has been minimized.

Copy link
@Cadair

Cadair May 10, 2018

Author Member

This needs to not be defined if ASTROPY_SETUP is true.

return []

@keyword(False)
def figure(self, figure, kwargs):

This comment has been minimized.

Copy link
@Cadair

Cadair May 10, 2018

Author Member

need to add a figure_only

pre, post = self._generate_coverage_commands()
cmd_pre += pre
cmd_post += post

cmd = ('{cmd_pre}{0}; import {1.package_name}, sys; result = ('

This comment has been minimized.

Copy link
@Cadair

Cadair May 10, 2018

Author Member

need to make sure this list is upto date with the one in astropy.

@Cadair Cadair force-pushed the Cadair:test_runner branch from 464022a to 28bf727 May 13, 2018

@Cadair

This comment has been minimized.

Copy link
Member Author

commented May 13, 2018

Online test fails looks to be because of a dead NOAA server.

@sunpy/sunpy-maintainers review please!

@Cadair Cadair added the [Review] label May 13, 2018

@@ -75,3 +75,4 @@ $RECYCLE.BIN/
.hypothesis/
.pytest_cache/
sunpydata.sqlite
v/

This comment has been minimized.

Copy link
@nabobalis

nabobalis May 14, 2018

Contributor

What is this?

This comment has been minimized.

Copy link
@Cadair

Cadair May 14, 2018

Author Member

🤷‍♀

@@ -58,20 +58,12 @@ matrix:
# Order of tests run in build stages
- python: 3.6
stage: Initial tests
env: SETUP_CMD='test --coverage'

This comment has been minimized.

Copy link
@nabobalis

nabobalis May 14, 2018

Contributor

So what does this run now?

This comment has been minimized.

Copy link
@Cadair

Cadair May 14, 2018

Author Member

the same thing, it just uses the default value

This comment has been minimized.

Copy link
@nabobalis

nabobalis May 14, 2018

Contributor

Oh that was the default value already. Nevermind.

@@ -480,13 +480,13 @@ year:
>>> for database_entry in database:
... if database_entry.observation_time_start.month in [3,4,5]:
... database.tag(database_entry, 'spring')
>>> database.tags
>>> database.tags # doctest: +SKIP

This comment has been minimized.

Copy link
@nabobalis

nabobalis May 14, 2018

Contributor

Why were these tests skipped?

@@ -34,6 +34,8 @@ following into your interactive Python shell: ::
>>> import sunpy.data.sample # doctest: +REMOTE_DATA
>>> my_timeseries = ts.TimeSeries(sunpy.data.sample.GOES_XRS_TIMESERIES, source='XRS') # doctest: +REMOTE_DATA

.. doctest-skip-all

This comment has been minimized.

Copy link
@nabobalis

nabobalis May 14, 2018

Contributor

But why? :(

This comment has been minimized.

Copy link
@Cadair

Cadair May 14, 2018

Author Member

because they don't work!!

This comment has been minimized.

Copy link
@nabobalis

nabobalis May 14, 2018

Contributor

BUT HOW? I THOUGHT IT PASSED ON TRAVIS.

WHY DO WE HAVE AN ENTIRE FILE THAT HAS NO WORKING TESTS?

This comment has been minimized.

Copy link
@Cadair

Cadair May 14, 2018

Author Member

Yeah I have no idea how this was passing before! This file needs updating to fix the tests but this PR is not the time.

@@ -163,7 +159,7 @@ def time_query(self, start_time, end_time, table=None, max_records=None):
>>> hc = hec.HECClient() # doctest: +REMOTE_DATA
>>> start = '2005/01/03'
>>> end = '2005/12/03'
>>> temp = hc.time_query(start, end, max_records=10) # doctest: +REMOTE_DATA
>>> temp = hc.time_query(start, end, max_records=10) # doctest: +REMOTE_DATA +SKIP

This comment has been minimized.

Copy link
@nabobalis

nabobalis May 14, 2018

Contributor

Skip?! :(

This comment has been minimized.

Copy link
@Cadair

Cadair May 24, 2018

Author Member

this one also needs user input.

@@ -223,7 +219,7 @@ def make_table_list(self):
--------
>>> from sunpy.net.helio import hec
>>> hc = hec.HECClient() # doctest: +REMOTE_DATA
>>> hc.make_table_list() # doctest: +REMOTE_DATA
>>> hc.make_table_list() # doctest: +REMOTE_DATA +SKIP

This comment has been minimized.

Copy link
@nabobalis

nabobalis May 14, 2018

Contributor

But my skip?!

This comment has been minimized.

Copy link
@Cadair

Cadair May 24, 2018

Author Member

this one is real. It's just nonsense.

This comment has been minimized.

Copy link
@Cadair

Cadair May 24, 2018

Author Member

oh that's it, this function needs user input, so it can't be run as a doctest.

@Cadair

This comment has been minimized.

Copy link
Member Author

commented May 17, 2018

oh balls: https://travis-ci.org/sunpy/sunpy/jobs/378357403#L2557

The -k remote_data in the online build skips all the damn doctests

@Cadair Cadair force-pushed the Cadair:test_runner branch from 28bf727 to b9d3e06 May 24, 2018

Cadair added some commits Feb 1, 2017

@Cadair Cadair force-pushed the Cadair:test_runner branch from 3fa7499 to 689d7f3 May 26, 2018

@Cadair

This comment has been minimized.

Copy link
Member Author

commented May 26, 2018

Assuming my rebase hasn't broken anything this is now good to go. Reviews please!

@nabobalis
Copy link
Contributor

left a comment

Looks like python code.

@Cadair Cadair merged commit dc48228 into sunpy:master May 28, 2018

9 checks passed

Giles Click details to preview the documentation build
Details
ci/circleci: egg-info-27 Your tests passed on CircleCI!
Details
ci/circleci: egg-info-35 Your tests passed on CircleCI!
Details
ci/circleci: egg-info-36 Your tests passed on CircleCI!
Details
ci/circleci: html-docs Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 76.34%)
Details
codecov/project 83.5% (+7.15%) compared to 0d784d2
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Cadair Cadair deleted the Cadair:test_runner branch May 28, 2018

@nabobalis nabobalis moved this from High Priority Features to Finished in SunPy 1.0 Sep 24, 2018

nabobalis added a commit to nabobalis/sunpy that referenced this pull request Jan 17, 2019

Merge pull request sunpy#1983 from Cadair/test_runner
Update the test runner to use the updated code in astropy 1.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.