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

Sample data test infrastructure #20

Merged
merged 5 commits into from
Aug 22, 2024

Conversation

j-emberton
Copy link
Member

@j-emberton j-emberton commented Aug 16, 2024

Have added some basic infrastructure to get up and running with domain specific integration level tests:

Added matlab engine pytest fixture that can be configured to run matlab code from the original or refactored code repo.

Added fixture to read canonical test case data into temporary dirs for testing

Added /test_artifacts/ dir within /tests/ to be a home for reference test data.

Added the input and output files associated with the US_ARc_sample_data.

The idea here is that any individual test can demand a fixture which sets up a specific set of reference data for that test

This should form the backbone of testing against domain data unit and regression tests going forward

@j-emberton j-emberton self-assigned this Aug 16, 2024
@j-emberton
Copy link
Member Author

@dorchard @isaacaka @tztsai

I've setup the matlab engine envisaging that we have two parallel sets of matlab code - 'ustar_cp' and 'ustar_cp_refactor_wip'. The latter would initially be non-refactored matlab code, but over time would be incrementally refactored as we work on it.

This allows us to work from root to leaf as discussed yesterday with the lower level dependencies remaining intact while we refactor the higher level functions.

I'm hoping with this approach Matlab doesn't get confused with duplicate function names on its path - I haven't tested it yet.

This specific issue wouldn't affect the bulk of the implementation in this PR however.

If you are ok for us to test this approach I'll create the 'ustar_cp_refactor_wip' dir and migrate the matlab code into it.

Copy link
Member

@dorchard dorchard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments around documentation and a minor tweak but otherwise looks good. I think we probably also need a requirements.txt as well for the tests, or to extend the top-level one, to include matlabengine and shutil?

tests/conftest.py Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@isaacaka
Copy link
Member

Looks good, left a comment on using shutil.copytree()

@j-emberton
Copy link
Member Author

Looks good, left a comment on using shutil.copytree()

Good shout

@j-emberton j-emberton marked this pull request as ready for review August 20, 2024 12:51
@j-emberton
Copy link
Member Author

A few comments around documentation and a minor tweak but otherwise looks good. I think we probably also need a requirements.txt as well for the tests, or to extend the top-level one, to include matlabengine and shutil?

I've extended the top level one for the time being, though matlab engine is only needed transiently. I think shutil is part of python standard lib so no need for an entry in requirements.txt.

@j-emberton
Copy link
Member Author

OK, made the changes you guys suggested and created the duplicate matlab dir. Good to go I think.

Copy link

@tztsai tztsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In PR #19 you also created a conftest.py file, which one has a newer version?

tests/conftest.py Show resolved Hide resolved
@dorchard
Copy link
Member

Can we have a README.md in the tests directory that explains the requirements and workflow for running the tests? Also which version of Python should these be using wrt. the requirements? @j-emberton

@j-emberton
Copy link
Member Author

Can we have a README.md in the tests directory that explains the requirements and workflow for running the tests? Also which version of Python should these be using wrt. the requirements? @j-emberton

Yes. But it might fit better in the launch.m PR where I've actually setup the test env and have running tests. Otherwise the readme will refer to stuff that doesn't exist yet.

@j-emberton
Copy link
Member Author

In PR #19 you also created a conftest.py file, which one has a newer version?

You're right. This one is the latest one.

@dorchard
Copy link
Member

(Note should work with Python 3.11)

@dorchard dorchard merged commit bbe0b0c into ustar_cp_refactor_master Aug 22, 2024
@dorchard dorchard deleted the sample_data_test_infrastructure branch August 22, 2024 13:30
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.

4 participants