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

Re-organize tests/fmf-plans into a more concise format #11809

Merged
merged 7 commits into from
Apr 17, 2024

Conversation

comps
Copy link
Collaborator

@comps comps commented Apr 10, 2024

Description:

  • This is about cleaning up and re-organizing tests/fmf-plans/ into what I feel is a more concise and organized form.

  • The end result should be nicely named TMT plans in Testing Farm (without /tests/fmf-plans prefix), and with TMT HTML reports available in the Testing Farm data dir structure

    • Unfortunately, there's no easy way to expose the path to this HTML on the Packit GH check page.
  • Note that this brings in a testing change - currently, some Beakerlib-based tests from fedoraproject.org run on Fedora via Testing Farm

    • Sanity/ansible-allowed-modules
    • Sanity/smoke-test (rpmbuild + CTest)

    these would not be running on Fedora anymore, where Contest is not supported. However, given their nature, I believe running them on CentOS Stream is sufficient.

    • @mildas @matusmarhefka Please comment whether this is okay.
    • If so, then this PR also requires the removal of testing-farm:fedora-39-x86_64 from the "required" Github checks.

Review Hints:

  • See individual commits, they usually have descriptions detailing the rationale for each change.

There are only a few test cases in the fedoraproject repo,
all of them now abandoned due to the same logic being in Contest:

Sanity/ansible-allowed-modules
 --> /static-checks/ansible/allowed-modules
Sanity/smoke-test
 --> /static-checks/rpmbuild-ctest

The two remaining fedoraproject tests (not run in Packit) are also
covered by Contest:

Sanity/ansible-machine-hardening
 --> /hardening/host-os/ansible
Sanity/machine-hardening
 --> /hardening/host-os/oscap

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
All of these are running Contest anyway, might as well use
a more conscise definition format, and put it into one file.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
FMF has no concept of 'plans', that's a TMT feature.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
This changes the TMT plan names from ie.

  /tests/tmt-plans/some/name

to just

  /some/name

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
There are no remaining FMF/TMT tests that would run on Fedora.

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
Signed-off-by: Jiri Jaburek <comps@nomail.dom>
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@mildas
Copy link
Contributor

mildas commented Apr 11, 2024

The changes look fine and no problem on my side in regards of not running tests on Fedora anymore.

However, the html-links tests seems quite flaky for GH CI, that's something to solve before merging this PR. I've also noticed it in productization that some links time out but are fine in re-run. And then there's the certificate problem which might be temporary issue but still it would block PRs.
Thus, I wonder if we want to have html-links enabled in CI. Content contains a lot of links a it might happen quite often that one link here or there will fail because of some temporary external problem.

@comps
Copy link
Collaborator Author

comps commented Apr 11, 2024

The changes look fine and no problem on my side in regards of not running tests on Fedora anymore.

👍

However, the html-links tests seems quite flaky for GH CI, that's something to solve before merging this PR. I've also noticed it in productization that some links time out but are fine in re-run. And then there's the certificate problem which might be temporary issue but still it would block PRs. Thus, I wonder if we want to have html-links enabled in CI. Content contains a lot of links a it might happen quite often that one link here or there will fail because of some temporary external problem.

Then we should probably just get rid of the test. Something that fails frequently enough that it would cause issues in pull requests, and is as insignificant as broken HTTP links in text, is not worth the combined days/weeks of human time investigating the failures.

@mildas
Copy link
Contributor

mildas commented Apr 12, 2024

Are you fine with excluding the html-link test from here, then we can also exclude it from daily productization and run it only in stabilization test runs?

Signed-off-by: Jiri Jaburek <comps@nomail.dom>
@comps
Copy link
Collaborator Author

comps commented Apr 12, 2024

Are you fine with excluding the html-link test from here, then we can also exclude it from daily productization and run it only in stabilization test runs?

I've pushed a commit disabling it here, for now, to not block #11807 or whatever new PR I'll make with those changes. .. We can adjust the exclusion later if needed.

Copy link

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:11809
This image was built from commit: 1bd04d6

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:11809

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:11809 make deploy-local

Copy link

codeclimate bot commented Apr 12, 2024

Code Climate has analyzed commit 1bd04d6 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 59.2% (0.0% change).

View more on Code Climate.

@mildas
Copy link
Contributor

mildas commented Apr 12, 2024

As RHSecurityCompliance/contest#148 got merged, now we need to wait until #11796 is merged

@Mab879 Mab879 self-assigned this Apr 16, 2024
@Mab879 Mab879 added this to the 0.1.73 milestone Apr 16, 2024
@Mab879 Mab879 added the Infrastructure Our content build system label Apr 16, 2024
@Mab879
Copy link
Member

Mab879 commented Apr 16, 2024

/packit build

Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for the work.

@Mab879 Mab879 merged commit 3581b3f into ComplianceAsCode:master Apr 17, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants