-
Notifications
You must be signed in to change notification settings - Fork 189
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
Update test-suite #1758
Update test-suite #1758
Conversation
@adrianreber Does the CI have a routine for generating the doc and test-suite tarballs needed to validate these packages? |
%if 0%{?suse_version} | ||
if [ $(getent group singularity) ]; then | ||
usermod -a -G singularity ohpc-test | ||
fi | ||
%endif | ||
exit 0 | ||
if [ $(getent group geopm) ]; then | ||
usermod -a -G geopm ohpc-test |
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.
usermod -a -G geopm ohpc-test | |
usermod -a -G geopm %{testuser} |
%undefine __brp_mangle_shebangs | ||
|
||
%if 0%{?suse_version} | ||
%if 0%{?suse_version} || 0%{?sle_version} |
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.
From what I saw in rpm --showrc
checking for %{?sle_version}
should be enough I think.
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.
I think you're correct, but I'm going by the chart here:
https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto
It implies that someone running Tumbleweed requires a suse_version check. I know it's not formally supported, but I still want it to work.
I've been kicking around adding %{suse} to OHPC_MACROS as the counterpart to %{rhel}.
Requires(pre): shadow | ||
Requires: python-base | ||
Requires: python3-base | ||
%endif | ||
|
||
%if 0%{?rhel_version} |
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.
Can you also change this to 0%{?rhel}
. This is one of the OBS specific macros which are not available outside of OBS.
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.
Good catch. I'll do that.
This should at least be three commits. One for the spec file changes. One for the removal of unused tests and one for the 2to3 conversion (thanks for looking at that).
Unfortunately not yet. I thought about having something for those cases, but currently we do not. Would be good to have. We would need a hook in the scripts somewhere which detects the documentation spec file and test-suite spec file and then create the tarballs during CI runs. Do you want to try to implement something which could help us here? |
I think it's funny because I usually get told to squash my commits in a PR (even if they're not just to fix a previous commit). Since it's all in a single commit, undoing and splitting should be simple. I'll also look at the tarball generation. One other thing I'd like to do is move the whole test suite to /opt/ohpc and not create a user. I'd like to leave the test-user setup to the cluster admin and also allow other users to access to tests. I won't make that change in this PR. |
I am usually asking to squash fix up commits. If you are fixing things in a PR then we do not need that in the git history if we found it during the review. If you have multiple logical things in your PR like here, then it should be in separate commits. Especially if we later need to look at the history it makes it easier to understand the change and possibly revert it. |
Not sure if this doable when looking at Jenkins. Not sure how Jenkins ingests the test suite. |
Signed-off-by: jcsiadal <[email protected]>
Signed-off-by: jcsiadal <[email protected]>
Signed-off-by: jcsiadal <[email protected]>
Signed-off-by: jcsiadal <[email protected]>
b65745f
to
711e072
Compare
Signed-off-by: jcsiadal <[email protected]>
It's all working (at least the build/install). The failures on RHEL/CentOS are happening because our autoconf package is missing dependencies. The autoconf build dependencies are set, but the install dependencies are missing. I'll issue a separate PR. |
A friendly reminder that this PR had no activity for 30 days. |
Update test package
More work to do on individual tests, but this allows it to build and configure on SLES15.4.
Signed-off-by: jcsiadal [email protected]