Add unit and integration tests for Slurm #2233
Conversation
| 15994954.1 2025-02-24T16:12:17 2025-02-24T16:12:17 2025-02-24T16:21:22 00:09:05 844.90K | ||
| ''') | ||
| tuple_data = slurm_platform.parse_job_finish_data(output, packed=False) | ||
| assert tuple_data is not None |
There was a problem hiding this comment.
Note to self, get the correct output from Sergi, then update the string above, and continue writing the test to actually test something, not is not None 😬
There was a problem hiding this comment.
To push local changes
There was a problem hiding this comment.
Huh, didn't find the code I had written on my laptop at home. Will take a look at my other BSC laptop next week 😰 (or will have to write the test again 😥 )
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2233 +/- ##
==========================================
+ Coverage 52.10% 52.34% +0.24%
==========================================
Files 71 71
Lines 17610 17617 +7
Branches 3416 3417 +1
==========================================
+ Hits 9176 9222 +46
+ Misses 7635 7590 -45
- Partials 799 805 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I see this assigned to myself, but I also see that this is a draft .. so what is the real status of this one? |
kinow
force-pushed
the
add-some-slurm-tests
branch
from
25912d9 to
4e9d2b0
Compare
kinow
force-pushed
the
add-some-slurm-tests
branch
from
4e9d2b0 to
d104867
Compare
kinow
changed the title
|
@VindeeR I've added two integration tests. One creates an experiment and the Slurm platform object, and can be used to check the initial state of the Slurm object. The other one is more interesting. If you modify the values for scratch dir (/gpfs/scratch), group (bsc32), and user (your user), and host to glogin?.bsc.es, the test should run fine against BSC MN5. If your PR, #2362 works, we can just point the test to the Slurm container, and the GitHub Actions should then pass. |
|
I will update the unit test that I wrote using performance data. I had written it in some branch, in some laptop, but I cannot find it 😅 So I will have to rewrite it tomorrow or over next days when I take a break from Postgres. |
| @@ -153,20 +149,18 @@ def process_batch_ready_jobs(self, valid_packages_to_submit, failed_packages: li | |||
| except AutosubmitError as e: | |||
| job_names = [] | |||
| duplicated_jobs_already_checked = True | |||
| try: | |||
| with suppress(Exception): | |||
There was a problem hiding this comment.
Besides suppress, nothing exciting to see here. Mainly fixing IDE/linter issues in comments, code, types, ...
| }) | ||
|
|
||
| exp.autosubmit._check_ownership_and_set_last_command(exp.as_conf, exp.expid, 'run') | ||
| assert 0 == exp.autosubmit.run_experiment(_EXPID) |
There was a problem hiding this comment.
Note to self: maybe add to AutosubmitExperiment a function run that does both, check ownership + run?
|
|
||
|
|
||
| def _create_slurm_platform(as_conf: AutosubmitConfig): | ||
| return SlurmPlatform(_EXPID, _PLATFORM_NAME, config=as_conf.experiment_data, auth_password=None) |
There was a problem hiding this comment.
Note to self: We should create the platform dictionary here, so it's not repeated in each test; the problem is that we may have to call as_conf.save(), or sort out what's called first... I tried it today and got the wrong data saved to disk.
# This is the 1st commit message: build a slurm container where itll replicate the systems behaviour allowing for more complex testing of the autosubmit tool facilitating the addition of coverage to the report file ensuring quality of the code # The commit message #2 will be skipped: # adding changes introduced by @kinow at the PR #2233 # The commit message #3 will be skipped: # adding extra testing to bump up the coverage of the slurm # The commit message #4 will be skipped: # f
|
Superseded by #2362, thanks @manuel-g-castro & @VindeeR for the work on the image, GH Actions & tests ! ⭐ |
Partially addresses #977
Temporary branch, that I will either update or delete in the next days.
@sergipalomas commented about an issue on how energy is reported by the API, but looking at AS code, it seems that AS is the one actually storing
0in theenergycolumn of job data.BSC-ES/autosubmit-api#175 (comment)
I started writing a unit test for that, but I realized that the
sacctoutput I have is missing some columns. I have jobs with energy consumption, but I'm interested in using Sergi's example to check if I can reproduce the case whereenergy=0(then I can get that and write a test, and get another one that doesn't return0, covering both -- this is, if we confirm that it is indeed0and not another value that should be returned).@VindeeR & @isimo00 will also work on the Slurm platform soon for Maestro (#2165), and we will have to increase our test coverage for that module, so it'll be a win-win for all if we can get this test written and working.