-
Notifications
You must be signed in to change notification settings - Fork 235
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
Should the new SIPNET test run on every PR or should it be scheduled on weekly basis #3184
Comments
Is it possible to set it to run daily but to skip if there's been no change to the code? If not, weekly is probably good. Would just want to make sure that the notification for the status goes somewhere that will actually have an impact. The only advantage of the PR option is that it blocks things until it is fixed. If a failed integration test just sends an email that gets ignored then its not doing much. |
I also wanted to point that this test would not be limited to just the sipnet model, but in future different models would be tested, for that I have added the matrix option where we can add different parameters like site_id, mode_id, etc to test against the docker stack, and all these tests can run in parallel. So, I think that keeping it with the PR option might not be helpful. Also can we go for the weekly option? Because When the PR's will get merged and the latest images will get uploaded, then we can pull the latest images and test against them. Currently we make a GET request to display the output of the sipnet test |
@RohanSasne with your proposal to test against a matrix of different models, sites, etc, I wanted to double check that you are UPDATING the existing suite of integration tests not REINVENTING that suite. Also, for the specific test you linked to above, Niwot Ridge is not a tropical forest. There's nothing gained by testing site x PFT combinations that aren't plausible. |
Opps, I had used those parameters as those were mentioned in the documentation to test the workflow , Can you please provide me with the correct parameters to test the full run. Does this file contain the correct parameter? Edit: I actually tried with the parameters mentioned in the file above and the test was successfull on my localhost, updating the latest changes on my PR, will update this comment with the link shortly |
Hey @mdietze, The parameters are now according to the file i Referred above, can you please have a look at the workflow now
The current use of matrix will create a new job altogether for every Matrix element(unique job for each test parameters) and it will also need to pull and build the whole docker stack again, if we do not want to build it again then a option would be to make a script file consisting of the cURL requests to run our models, and then we can just append different models in the script file, So should we proceed with that direction or the current matrix jobs are fine? |
@RohanSasne Niwot ridge is a conifer site Also, on the issue of a matrix of jobs, I'm not saying that you can't modify how things are done, I'm saying you can't duplicate how things are done. Having two different jobs running the same tests doesn't work. Having two different code bases do the same thing doesn't work. If there's pros/cons of different approaches it would be good to lay those out BEFORE you reinvent something |
Agree with you sir. The only benefit of running a matrix would be that we can save sometime to test full run when we add more models to it, but again it would be much more resource intensive, so for now I think that it's better to run a script file than having matrix. So should I update the PR accordingly @mdietze ? |
Hey, @mdietze , The PR is up for review, can you please have a look whenever you get free :) |
Description
Is your feature request related to a problem? Please describe.
As part of GSoc Project, We are creating a workflow which will test the current SIPNET model against the full docker stack brought up by docker compose.
It is resource intensive as we pull all the images in the docker compose and build them.
The testing workflow can be found in the draft PR: #3183
Proposed Solution
So should we schedule it for weekly basis or let it run on every PR ?
I was of the opinion to run it weekly, but open to the suggestions from the community.
cc. @robkooper @mdietze @infotroph
The text was updated successfully, but these errors were encountered: