Fixing sol interface tests

[cmhyett]

Checklist

• Appropriate tests were added
• Any code changes were done in a way that does not break public API
• All documentation related to code changes were updated
• The new code follows the
  contributor guidelines, in particular the SciML Style Guide and
  COLPRAC.
• Any new documentation only uses public API

Additional context

It was clear how to remedy the first test by including the periodic BC data that the ODE solver doesn't know.
I was not sure what the second test was attempting, open for suggestions. Atm just marked as broken.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (494dd7e) 0.00% compared to head (b35321b) 84.74%.

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #357       +/-   ##
===========================================
+ Coverage    0.00%   84.74%   +84.74%     
===========================================
  Files          37       41        +4     
  Lines        1910     1960       +50     
===========================================
+ Hits            0     1661     +1661     
+ Misses       1910      299     -1611     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cmhyett]

PR ballooned a bit. Recent(?) change to Test.jl must've changed the functionality of @test_broken so that declaring it before a testset now throws an error, e.g. @test_broken begin ... end around tests now errors with Expression evaluated to non-Boolean

Initially I was simply going to find/replace with @test_skip, but @test_broken is nice when it is on specific tests. So I attempted to only use @test_skip when there was an internal error being thrown in a "broken" testset, and otherwise reinstate the @testset with @test_broken tests.

We ought to track and remedy the skipped tests.

[ChrisRackauckas]

It looks like @test_broken was being used incorrectly. We need to mark the specific lines in there as being broken, and any broken needs to be an error or bool. It should be @test_broken SciMLBase.successful_retcode(solve(prob))

[ChrisRackauckas]

Skipping the full testsets seems a little dangerous because then we get no warning of them being fixed, which is what I assume we want to be doing soon

[ChrisRackauckas]

PR ballooned a bit. Recent(?) change to Test.jl must've changed the functionality of @test_broken so that declaring it before a testset now throws an error, e.g. @test_broken begin ... end around tests now errors with Expression evaluated to non-Boolean

Yes that was a v1.10 change.

[cmhyett]

• replace sol.retcode == ... with SciMLBase.successful_retcode(solve(prob))
• Find offending line in skipped testsets, add in @test_throws, no skipped testsets.

[ChrisRackauckas]

Find offending line in skipped testsets, add in @test_throws, no skipped testsets.

@test_throws isn't the correct thing to use here. @test_throws is made to test a correct error throw, i.e. setting up a deliberate error and catching that the correct error is found. In these situations, the tests are throwing when they shouldn't be. That should be using @test_broken, but changing the call to evaluating a boolean that will check for if the test ever passes. So on the prob one, you'd do @test_broken ODEProblem(...) isa ODEProblem