-
Notifications
You must be signed in to change notification settings - Fork 6
Issue #1373 add test plan #1545
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
Conversation
deltamarnix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just the start of the document. The setup is good enough, but details are lacking for a tester to be able to check off boxes while testing.
deltamarnix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still some missing links and instructions.
docs/developing/test-plan.rst
Outdated
| same as iMOD5 (accounting for differences in row sorting.), unless there was a | ||
| conscious decision to divert from this. | ||
| * The conversion from projectfile to MODFLOW6 and MetaSWAP input files should be | ||
| done in a reasonable amount of time and should not be much slower than iMOD5. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is too vague. Add some percentages like:
| done in a reasonable amount of time and should not be much slower than iMOD5. | |
| compared to the same conversion process of iMOD5. Differences in performance may not exceed 5%. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, are the iMOD5 tests run for comparison on the same machine? Add instructions on how to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added both.
Fixes #1570 # Description - Adds pixi tasks which write away data a model dump to MDAL. This is part of the [test plan](#1545), to be imported in QGIS and test if this still works. - Fixes bug where error was thrown when attempting to regrid or mask a variable with only a layer dimension. # Checklist <!--- Before requesting review, please go through this checklist: --> - [x] Links to correct issue - [x] Update changelog, if changes affect users - [x] PR title starts with ``Issue #nr``, e.g. ``Issue #737`` - [x] Unit tests were added - [ ] **If feature added**: Added/extended example
|
|
UPDATE: The user acceptance data (LHM stress test) job is now finished. I versioned the data using DVC, so that changes to the data can be committed to the git repo, without having to push the actual data to Github. Instead, it's pushed/pulled from a Deltares MinIO storage. I updated the instructions accordingly, increasing clarity a lot. |
deltamarnix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work. I think this is definitely going to help future testers to perform a whole bunch of tests.
Some things can likely still be automated, E.g., running the user acceptance tests can be done on Teamcity, and let testers only download the output to verify if it is correct?
| * The tests should run without warnings from iMOD Python, unless unavoidable. | ||
| * The conversion of the transient LHM model run of 1 year on a daily timestep | ||
| (365 stress-periods) should run without memory overflow on a machine with 32 | ||
| GB and write a model within 15 minutes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the tester test this by themselves, or does the output tell you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output tells you that! It needs to be run on a 32 GB ram machine though, but I think the instructions are clear enough here?
| Build the documentation locally by running the following command in the root of | ||
| the repository: | ||
|
|
||
| .. code-block:: console | ||
|
|
||
| pixi run docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should thisi be done locally? Isn't it possible to just download the files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs are published online only when creating a new release. This step is to test the docs before releasing, so better to do it locally I'd say.
|



Fixes #1373
Description
Adds test plan for the 1.0 release. Two remarks:
UPDATE:
Checklist
Issue #nr, e.g.Issue #737