Update tests + CI, remove cruft, fix a bug #23
Conversation
* use GHA for CI * disable documenter for now
* rework tests * coverage * drop Julia 1.0
|
|
||
| [deps] | ||
| DeepDiffs = "ab62b9b5-e342-54a8-a765-a90f495de1a6" | ||
| Distributed = "8ba89e20-285c-5b6f-9357-94700520ee1b" | ||
| Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40" | ||
|
|
||
| [compat] | ||
| julia = "1" | ||
| Aqua = "0.8" | ||
| DeepDiffs = "1" |
There was a problem hiding this comment.
I'm glad DeepDiffs is still up and running! I haven't touched it in years.
test/no_diff.jl
Outdated
| end | ||
| end | ||
| end | ||
| @test contains(output, "Diff:\nnothing") |
There was a problem hiding this comment.
this one's weird. It's been a while since I've looked at this package so I don't remember what the expected behavior is here. It doesn't seem like Diff: nothing would be what you'd want though, given that there is a difference. If Matrices aren't supported by DeepDiffs, I'd think this just wouldn't include the Diff: at all.
probably worth a comment to explain.
There was a problem hiding this comment.
Yeah. Maybe the better approach is
- disable diffing on matrices for now
- check out what DeepDiffs is up to
- update to include matrix functionality
There was a problem hiding this comment.
okay, the issue was more interesting than I thought -- there is an implicit else branch that led to a nothing in the logic.
| Logging = "56ddb016-857b-54e1-b83d-db4d58db5568" | ||
| MetaTesting = "9e32d19f-1e4f-477a-8631-b16c78aa0f56" |
There was a problem hiding this comment.
How much do we gain from MetaTesting? It doesn't look like the EncasedTestSet type you're using here is documented. AFAICT it is supposed to isolate the wrapped tests, but we still end up wrapping them in try...catch blocks.
I'm not strongly opposed, if you think it makes things easier. I just want to make sure there's a good reason to add an additional dependency.
There was a problem hiding this comment.
It made some of the testing easier -- without MetaTesting, the blocks were even more of an mess. It's "just" a test dep, so it doesn't hurt most users.
I can also set the compat entry for MetaTesting to exact equality, since we are reaching into internals a bit.
There was a problem hiding this comment.
sounds good. just wanted to make sure. re: making the compat entry exact, I'd say it's not necessary, but is good to keep in mind if things break in the future.
There was a problem hiding this comment.
whoops, didn't check the rest of the new commits before commenting. I'm good with it either way.
|
@ssfrr can you enable the CodeCov integration/app/whatever GH calls it for this repo? |
That's a lot of churn, but each commit should be more or less self-contained.