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 failing tests #623

Merged
merged 12 commits into from
Dec 18, 2019
Merged

Fix failing tests #623

merged 12 commits into from
Dec 18, 2019

Conversation

lrennels
Copy link
Collaborator

@lrennels lrennels commented Dec 3, 2019

reminder to myself: get all tests passing without doctesting explore and plot and then try to put those back in in another PR...

@lrennels lrennels closed this Dec 3, 2019
@lrennels lrennels reopened this Dec 3, 2019
@lrennels lrennels changed the title Fix tests failing on v1.3 Fix failing tests Dec 3, 2019
@davidanthoff
Copy link
Collaborator

Joy, tests fail only on 32 bit systems, apparently :)

@lrennels
Copy link
Collaborator Author

lrennels commented Dec 7, 2019

Ugh, it's all these subtle differences in the output of doctests ... I'll work on it.

@davidanthoff
Copy link
Collaborator

This is sucking up a lot of time, though... I wish there was some way to run doctests, but only run them, not check their output. Or maybe that can be done? I think that would be good enough for us?

@lrennels
Copy link
Collaborator Author

lrennels commented Dec 8, 2019

I believe what Cora and I concluded is that we can’t use a flag, but most of the time can use a regular expression and the filter keyword arg to filter out all output and thus ignore output. We don’t do this in all cases, but that is probably going to be my solution now ...

@codecov
Copy link

codecov bot commented Dec 8, 2019

Codecov Report

Merging #623 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #623   +/-   ##
=======================================
  Coverage   80.12%   80.12%           
=======================================
  Files          39       39           
  Lines        2732     2732           
=======================================
  Hits         2189     2189           
  Misses        543      543

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e30eb0...278bdd3. Read the comment docs.

@davidanthoff
Copy link
Collaborator

@davidanthoff
Copy link
Collaborator

You probably saw this already: JuliaDocs/Documenter.jl#452 (comment)

@lrennels
Copy link
Collaborator Author

lrennels commented Dec 8, 2019

I haven't tried out the fixing outdated doctests yet, but yes we use the filter suggested in the second link to compare against nothing but still check if the test fails. I think this branch might be set now, but not sure because it's failing before it gets to the doctests due to some compat error.

I am curious about the outdated doctests. I think one issue is sometimes the output is slightly different on different machines and Julia versions, so sometimes it'll pass on my machine but fail one of the tests ... which is why I punted on some and just filtered the output so it's ignored.

@davidanthoff
Copy link
Collaborator

Alright, I think we are back to failing doctests?

@lrennels
Copy link
Collaborator Author

@davidanthoff I'm getting some frustrating and unpredictable stalling of tests that doesn't seem to be related to our stuff at all ... going to give it a day or two and see if it fixes. All of our tests should run fine, so this will get master passing again and allow us to properly test other PRs.

@lrennels lrennels merged commit 843c4de into master Dec 18, 2019
@lrennels lrennels deleted the tests branch December 18, 2019 23:39
@davidanthoff
Copy link
Collaborator

Wait, it all worked? Hurray!!! I thought this saga would never end ;)

@lrennels
Copy link
Collaborator Author

Well ... I thought it worked because it let me merge but now I'm worried master isn't going to work because I think two were still pending. Cross your fingers.

@lrennels
Copy link
Collaborator Author

The saga may continue. But I have no clue what is going on if it doesn't work, all the tests pass and I removed all the doctesting of plot and explore for now to keep things as clean as I could.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants