FluidityProject / fluidity

Fluidity
http://fluidity-project.org
Other
365 stars 115 forks source link

Fix tests examples #313

Closed drhodrid closed 3 years ago

drhodrid commented 3 years ago

The gcc10 build and migration to focal identified a number of problems with particle tests cases, that I've been tasked with rectifying. Fortunately, it seems that most (if not all!) of these issues arise because of poor test design. I am therefore in the process of updating a number of these tests. I think the best route forward is:

  1. I update all relevant test cases and propose a merge into master. This will ensure that our test suite remains stable on bionic.
  2. Those changes are then ported across to the gcc10/focal build, to see if any outstanding issues remain.

This pull request is premature (please don't review yet) - I'm mainly setting it up now so that the test suite can be triggered and I can verify that my changes have worked as expected.

tmbgreaves commented 3 years ago

Many thanks @drhodrid - shout if you need anything on the CI side of things.

drhodrid commented 3 years ago

Hi @tmbgreaves - I'm almost there with improving these tests. To make some more reliable I have had to increase mesh resolution. A consequence is that some tests will now need to become long tests. Could you update me on the status on longtests in Fluidity? Are they still run on every commit? Is there a way for me to test the new versions on both bionic and focal if they are long-tests?I guess one option is to run them through as medium tests for now, and then shift across post merge? Advice please!

tmbgreaves commented 3 years ago

@drhodrid At the moment longtests aren't routinely run, but at the point we have migrated to Actions I'd like to set up an on-demand builder to run at least some of the less demanding long tests on a regular basis, potentially on all PRs. Do you have a handle on how long your reworked tests will take to run on the github builders?

(It occurs to me that this may be a useful point to consider how to wrap long testing into Actions - I'll have a go at that over the next few days)

tmbgreaves commented 3 years ago

Starting the process of wrapping in longtests to the Actions PR in #314

tmbgreaves commented 3 years ago

First look at this suggests the longtest are rather out of date in terms of things like python3 safety and need some work. Some are passing, some passing quite quickly (so could probably move to mediumtesting) others are looking quite unhappy.

tmbgreaves commented 3 years ago

@drhodrid - I've added your four newly-long tests to the longtests repo and added them into the testing matrix in #311 . I'm still not sure this is the right way to do longtesting, but given the previous method had turned into a 'how broken is IC HPC's Fluidity setup' test I think it's a step forward and can at least get the majority of the longtests back into regular runs.

tmbgreaves commented 3 years ago

@drhodrid - running the four newly-long tests through Focal gives errors in all, see:

https://github.com/FluidityProject/fluidity/actions/runs/802838710

Those are running out of the longtests repo - if you update them in this branch/PR I can copy them over :-)

drhodrid commented 3 years ago

Thanks for the heads up Tim. I've just pushed a few changes, which will hopefully get these tests through. Sadly, I think one of the tests has highlighted an issue with particles (even on bionic!), which I'm looking into, but it will take some time to figure out. I'll likely need to involve @angus-g... More from us, when I get to the bottom of it. For now, I'm hoping these tests cases will turn green in light of the changes I just pushed, and thus the particle issue shouldn't hold up this merge.

tmbgreaves commented 3 years ago

Thanks Rhodri - I've shunted the four longer ones out to VLong testing - see https://github.com/FluidityProject/longtests/actions for work in progress on that.

tmbgreaves commented 3 years ago

Tests look green on bionic - the failure is the broken unittest being dealt with elsewhere. The new tests take a long time but get through green, so I'll merge in and then remove them from the mediumtest suite, and merge master back into the Focal fixes to see how things look there.