-
Notifications
You must be signed in to change notification settings - Fork 117
[feat] Add support for building tests using EasyBuild #1719
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
|
Hello @rsarm, Thank you for updating! Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide Comment last updated at 2021-03-08 20:43:55 UTC |
|
At this point, the test has to define the following: self.modules = ['EasyBuild-custom/cscs']
self.build_system = 'EasyBuild'
self.build_system.easyconfigs = [<easyconfigs>]
self.build_system.options = [<easybuild-cl-options>] |
boegel
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.
Just some feedback, let me know if I can be of any further help with this!
|
Thanks @boegel for reviewing! |
|
Not sure how you've set things up here (haven't looked closely enough) but I assume it's based on having a pre-existing EB installation with the toolchains and basic stuff? Or was the plan to have it build totally from scratch? And it needs to handle the existing EB using HMNS too. |
It assumes a pre-existing installation of EB.
Can you elaborate on this? |
|
I.e., if the existing EB uses HMNS any EB config params you use must make sure to also use it, and it needs to be done in such a way that the site installed EB's modulefiles that extend MODULEPATH can find the modules you install with ReFrame. Without subdir-user-modules you only get in the site module file, with subdir-user-modules=easybuild/modules you get without that if isDir and extra prepend_path, doing user installed EB modules with HMNS doesn't work. So, |
|
Thanks @akesandgren for the explanation. The initial idea behind this feature was not to install the package, but rather stay in the stage directory just for the duration of the test. See it as testing a recipe or using EB to install and test a more complex software. If the build phase also installs it in the final location, what would happen if the sanity or the performance break? Then your test would have an undesirable side effect. Is it possible in EB to "move" an installation? Because then, users could have a post-performance hook, that would install it to its final location. @boegel @victorusu ? |
|
That's what I mean by setting $HOME. So the package ReFrame installs using EB can easily be thrown away after by just removing the stage-dir where the actual install happened. You can't (normally) move a package installed by EB somewhere else though, due to path's usually getting upset... Any real-life site/user installation of the package would still have to be done outside of ReFrame, and that is, as far as I'm concerned, the way it should be. Does that explain things better? |
|
The initial idea was not to have ReFrame install the package and I'm also concerned about this, especially about the side effects I described above. We had a discussion with @rsarm and @victorusu today and we think that the best way is to build in the stage dir (unless the user really passes specific options to EB to change that, which I don't think is a good idea) and produce the necessary rpms (or any other supported packages from EB). Then, if the test is successful, you can keep this as an artifact with |
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.
We also need:
- Documentation for the EB build system
- Unit tests to test the
pathargument of the updated module interface - Unit tests for the EB build system.
- We still need to update the documentation in the configuration about defining modules.
|
Now the test should have |
Codecov Report
@@ Coverage Diff @@
## master #1719 +/- ##
==========================================
+ Coverage 87.60% 87.61% +0.01%
==========================================
Files 49 49
Lines 7940 8029 +89
==========================================
+ Hits 6956 7035 +79
- Misses 984 994 +10
Continue to review full report at Codecov.
|
- The `path` argument is now passed to any function that needs to create a module in the backend. - The `emit_load_instr` and `emit_unload_instr` backend functions are now returning a list of commands. - Unit tests for the `emit_load_commands` and `emit_unload_commands` were fixed. They are wrong in the master. - More unit tests were added to test explicitly for the `path` functionality.
Closes #300