Test launch.m - Githubissues

j-emberton commented 3 months ago

PR for testing and refactor of launch.m

progress to date:

added regression test framework for launch.m testing against reference outputs for chosen test cases
added unit tests which test against exitcode output for a variety of input scenarios
reworked some global fixtures in conftest to better suit test requirements
added pyproject.toml to setup desired pytest behaviour
tests run successfully

Decisions to be made:

do we want to extend range of unit tests by capturing stdout similar to regression test? Would allow greater test coverage. Do we want to extend print statements in launch.m to cover a greater range of scenarios?
Once we agree tests, I'll start the refactor

NB:

At the moment this branch is based on test_infrastructure branch rather than the ustar_cp_refactor_master branch. Once we merge the test_infrastructure PR into master, I'll rebase this branch to tidy things up. There's a lot of duplication in the commits and the diff just now which is a bit untidy.

j-emberton commented 3 months ago

Added coverage check. Doesn't actually work on the matlab code (only covers python and its tests) but will be useful once we start moving into Python

dorchard commented 3 months ago

Thanks for adding the extra documentation - this is looking good!

dorchard commented 3 months ago

Thanks for adding the extra documentation - this is looking good!

Sorry I added this to the wrong PR. Will look at this in depth now.

dorchard commented 3 months ago

Retargetting this PR to just be about the tests for launch.m rather than any refactoring yet. Now needs rebasing off ustar_cp_refactor_main

j-emberton commented 3 months ago

Rebased successfully

j-emberton commented 3 months ago

Going to aim to drop aim to refactor in this PR while we sort out integrating other test cases with ifs.

These tests make a decent baseline we can build on.

dorchard commented 2 months ago

I can confirm that everything is running fine for me now with the tests. What's not clear is whether this PR plans to include all the end-to-end data tests or not over launch.m? Or will we add that as a separate PR?

j-emberton commented 2 months ago

I can confirm that everything is running fine for me now with the tests. What's not clear is whether this PR plans to include all the end-to-end data tests or not over launch.m? Or will we add that as a separate PR?

Good to hear!

What do you mean by 'end-to-end data tests'? (Sorry, I might be being slow!).

The regression test has a single 'end-to-end' test but I'd agree this is incomplete as it doesn't give us good code coverage. I'd propose that we select out several cases to use as our 'reference' cases in a separate PR (partly base we have 400MB of test data and we maybe don't need all of it, just a small subset. This would also solve the issue of where do we store 400MB of data as git LFS isn't an option).

We should get all the end to end tests in place before thinking about merging any refactored code.

We might want to think about whether we want to do an initial refactor aimed at improving testability (i.e. get it to return more than 1 or 0) and a 2nd refactor to modularise it.

j-emberton commented 2 months ago

@isaacaka @tztsai

I hope to merge this PR in the next team meeting (12th Sept). Could you review this PR and make any suggestions in the next day or two so that we have time to implement and review any changes prior to the 12th.

isaacaka commented 2 months ago

This looks good to me. Nice work!

Cambridge-ICCS / ONEFlux

Test launch.m #21