Skip to content
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

Closed
wants to merge 5 commits into from
Closed

Update test-suite #1758

wants to merge 5 commits into from

Conversation

jcsiadal
Copy link
Contributor

@jcsiadal jcsiadal commented Mar 29, 2023

Update test package

  • Add config support for Rocky Linux 8
  • Remove Python 2 tests if they belong to removed packages
  • Update all remaining Py2 tests to Python3, using 2to3
  • Update specfile (remove Py2 deps and clean up).
  • Added generation of doc and test tarballs to get_sources. (also turned on error printing for build_srpm; log files aren't that useful if errors are being sent to null)

More work to do on individual tests, but this allows it to build and configure on SLES15.4.

Signed-off-by: jcsiadal [email protected]

@github-actions
Copy link

github-actions bot commented Mar 29, 2023

Test Results

0 files   -   4  0 suites   - 4   0s ⏱️ ±0s
0 tests  - 17  0 ✔️  - 17  0 💤 ±0  0 ±0 
0 runs   - 18  0 ✔️  - 18  0 💤 ±0  0 ±0 

Results for commit 435a349. ± Comparison against base commit 1dc6455.

♻️ This comment has been updated with latest results.

@jcsiadal
Copy link
Contributor Author

@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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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}
Copy link
Member

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.

Copy link
Contributor Author

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}
Copy link
Member

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.

Copy link
Contributor Author

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.

@adrianreber
Copy link
Member

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).

Does the CI have a routine for generating the doc and test-suite tarballs needed to validate these packages?

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?

@jcsiadal
Copy link
Contributor Author

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.

@adrianreber
Copy link
Member

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.

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.

@adrianreber
Copy link
Member

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.

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]>
@jcsiadal jcsiadal force-pushed the tests branch 6 times, most recently from b65745f to 711e072 Compare March 30, 2023 04:14
@jcsiadal
Copy link
Contributor Author

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.

Copy link

A friendly reminder that this PR had no activity for 30 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants