Skip to content

Ignore generated examples for pytest - #6228

Merged
bsipocz merged 1 commit into
astropy:masterfrom
saimn:pytest-generated
Jun 16, 2017
Merged

bsipocz merged 1 commit into
astropy:masterfrom
saimn:pytest-generated

Conversation

@saimn

@saimn saimn commented Jun 16, 2017

Copy link
Copy Markdown
Contributor

When the doc examples have been generated, pytest imports (and run) these files, included skip_create-large-fits.py which creates a 12Gb array in memory !

Also remove the astropy/sphinx setting as this directory was removed from Astropy a long time ago.

When the doc examples have been generated, pytest imports (and run)
these files, included `skip_create-large-fits.py` which creates a 12Gb
array in memory !

Also remove the `astropy/sphinx` setting as this directory was removed
from Astropy a long time ago.
@saimn
saimn requested a review from bsipocz June 16, 2017 12:50
@saimn

saimn commented Jun 16, 2017

Copy link
Copy Markdown
Contributor Author

I canceled the Travis build as I think it's not needed to run everything if it pass in CircleCI/AppVeyor.

@bsipocz

bsipocz commented Jun 16, 2017

Copy link
Copy Markdown
Member

Looks good to me. Merging as we've already tested this when tried to figure out the timeout issues in #5782

@pllim pllim added the testing label Jun 16, 2017
@bsipocz bsipocz added this to the v2.0.0 milestone Jun 16, 2017
@bsipocz
bsipocz merged commit ba4a02c into astropy:master Jun 16, 2017
@saimn
saimn deleted the pytest-generated branch June 16, 2017 12:54

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@pllim

pllim commented Jun 16, 2017

Copy link
Copy Markdown
Member

I don't disagree but I thought that solution didn't work for #5782 and then we ended up removing some Python file and embedded the code in RST?

@saimn

saimn commented Jun 16, 2017

Copy link
Copy Markdown
Contributor Author

I didn't follow the comments from #5782 , but this should not have an influence on Travis, which always starts with a clean environment.

@pllim

pllim commented Jun 16, 2017

Copy link
Copy Markdown
Member

this should not have an influence on Travis

Yes, I remember now. I was testing locally but the error went away after I did a git clean -xdf. Never mind me; Coffee still kicking in... ☕

@saimn

saimn commented Jun 16, 2017

Copy link
Copy Markdown
Contributor Author

To clarify: the problem here occurs only once you have run build_docs which generate the examples. Then pytest imports the examples, and execute the code in it.
Reading quickly #5782, the timeout issue was fixed by keflavich#11 I think, which was a similar issue: I guess pytest was importing and executing docs/convolution/images/onedconvolutionexample.py (though the array seems not so big ?)

@pllim

pllim commented Jun 16, 2017 •

Copy link
Copy Markdown
Member

Maybe something funky happens where pytest awaits some signal from the script that never arrives...

¯\(ツ)/¯

@saimn

saimn commented Jun 16, 2017

Copy link
Copy Markdown
Contributor Author

And there are other examples that are executed during pytest's collect phase: https://github.com/astropy/astropy/tree/master/docs/wcs/examples
from_file.py uses a if __name__ block so it's ok, but programmatic.py is clearly executed. So docs/wcs/examples should be added to the norecursedirs setting!

@bsipocz

bsipocz commented Jun 16, 2017 •

Copy link
Copy Markdown
Member

Anyway, I think it's good to have this in. It won't affect our CI as they start from scratch, but will be better for people running the tests after they've built the docs.

@pllim

pllim commented Jun 16, 2017

Copy link
Copy Markdown
Member

@bsipocz , indeed.

@saimn , are you sure that "programmatic.py" is not meant to be tested?

@saimn

saimn commented Jun 16, 2017

Copy link
Copy Markdown
Contributor Author

@bsipocz - sure, otherwise it's not funny when your laptop gets out of memory !

@pllim - ah maybe, as there is an assert it's possible.

@pllim

pllim commented Jun 16, 2017

Copy link
Copy Markdown
Member

@saimn , it's funnier on a Windows laptop. 😛

@saimn

saimn commented Jun 16, 2017

Copy link
Copy Markdown
Contributor Author

@pllim - Out of curiosity, what's the behavior on Windows for this case ?
On a linux laptop with 8Gb of RAM, it ran til the end, with the swap (and with Firefox and Chrome launched!).

@saimn

saimn commented Jun 16, 2017

Copy link
Copy Markdown
Contributor Author

Of course the laptop was not usable for a few minutes ! 😁

@pllim

pllim commented Jun 16, 2017

Copy link
Copy Markdown
Member

I am just imagining a blue screen. But for academic purpose, I can test this next week...

@pllim

pllim commented Jun 19, 2017

Copy link
Copy Markdown
Member

Turns out I can't even get the doc to build on Windows (astropy/sphinx-automodapi#28). "What a twist!"

@pllim

pllim commented Jun 20, 2017 •

Copy link
Copy Markdown
Member

So... after I hacked around the UnicodeDecodeError in automodapi and sphinx-gallery, running the test on generated examples (that you excluded here) for that large file one only gave me a MemoryError (my machine has 8 GB of RAM). Didn't hang for long and no blue screen!

Update: And just for completeness, with your fix on my Windows laptop, all the tests passed except for 6 remote data ones, that are probably due to connection issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants