Skip to content

Conversation

@jpodivin
Copy link
Contributor

@jpodivin jpodivin commented Nov 21, 2025

Unit tests, integration tests and database tests are now executed as github action. The workflow is, intentionally, very similar to what developer would have to use. And it should stay that way if possible.

@jpodivin jpodivin requested a review from a team as a code owner November 21, 2025 09:53
@gemini-code-assist
Copy link
Contributor

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@jpodivin jpodivin changed the title Adding workflow for tests DNM: Adding workflow for tests Nov 21, 2025
@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@softwarefactory-project-zuul
Copy link
Contributor

@jpodivin jpodivin changed the title DNM: Adding workflow for tests Adding workflow for tests Nov 21, 2025
@m-blaha m-blaha added the discuss To be discussed within team label Nov 24, 2025
@mfocko
Copy link
Member

mfocko commented Nov 27, 2025

As per the discuss label:

  1. This feels like further fragmentation of the CI setup Packit has (more specifically: pre-commit.ci, Zuul, Packit itself where it's feasible (all the way to the Testing Farm))

  2. In a sense it feels unnecessary, e.g., make check-in-container is what Zuul basically does.

    • except for make check-db… that part was deemed low-priority and was waiting for migration to TF
  3. If possible, I'd prefer utilising Testing Farm as has been proposed in ci(tmt): run the tests on Testing Farm #2443, but

    • the intention of aforementioned PR was to unify the testing CI across Packit, but it doesn't allow for reverse-dependency testing of Service itself, because of TFT-2521
    • there's been a considerable amount of work spent by @majamassarini on making the OpenShift setup “somewhat” work on Testing Farm; in the context of this PR we also discussed the possibility of using podman-compose on TF, which would probably require less effort to make it work in comparison to the OpenShift itself
  4. I appreciate the Build Test Image step, it would definitely make sense to me to:

    • run it for PRs, if any of the files related to the image build get changed, i.e., any of the Containerfiles and (most likely all of) files/ subtree
    • run it as part of the merge queue all the time, though this suggestion will probably be the most problematic, as the setup, right now, depends on Zuul and merging is also managed via Zuul.

There's also a possibility of trying out the TF setup even if the rev-dep tests (from Packit and ogr) are not possible right now, making the make check-db work with TF and decide on the next steps later based on the outcome of this.

Signed-off-by: Jiri Podivin <[email protected]>
@softwarefactory-project-zuul
Copy link
Contributor

@jpodivin
Copy link
Contributor Author

jpodivin commented Dec 1, 2025

As per the discuss label:

1. This feels like further fragmentation of the CI setup Packit has (more specifically: pre-commit.ci, Zuul, Packit itself where it's feasible (all the way to the Testing Farm))

I agree. I would personally drop pre-commit ci. It is already duplicating what is in zuul and provides little additional value.

2. In a sense it feels unnecessary, e.g., `make check-in-container` is what Zuul basically does.
   
   * except for `make check-db`… that part was deemed low-priority and was waiting for migration to TF

I've dropped the tests already executed in zuul. But database tests should be executed somewhere. Leaving them out, will inevitably lead to issues being discovered in stage or production.

3. If possible, I'd prefer utilising Testing Farm as has been proposed in [ci(tmt): run the tests on Testing Farm #2443](https://github.com/packit/packit-service/pull/2443), but
   
   * the intention of aforementioned PR was to unify the testing CI across Packit, but it doesn't allow for reverse-dependency testing of Service itself, because of [TFT-2521](https://issues.redhat.com/browse/TFT-2521)
   * there's been a considerable amount of work spent by @majamassarini on making the OpenShift setup “somewhat” work on Testing Farm; in the context of this PR we also discussed the possibility of using `podman-compose` on TF, which would probably require less effort to make it work in comparison to the OpenShift itself

In github, running things in compose requires only a minimal effort. Until the testing is done in TF, which may not be for a while yet, the tests should be run somewhere.

4. I appreciate the _Build Test Image_ step, it would definitely make sense to me to:
   
   * run it for PRs, **if** any of the files related to the image build get changed, i.e., any of the `Containerfile`s and (most likely all of) `files/` subtree
   * run it as part of the merge queue **all the time**, though this suggestion will probably be the most problematic, as the setup, right now, depends on Zuul and merging is also managed via Zuul.

There's also a possibility of trying out the TF setup even if the rev-dep tests (from Packit and ogr) are not possible right now, making the make check-db work with TF and decide on the next steps later based on the outcome of this.

@majamassarini majamassarini removed the discuss To be discussed within team label Dec 4, 2025
@majamassarini
Copy link
Member

We discussed this approach at the arch meeting last week and decided to prioritize work to unblock tests on Testing Farm. Therefore, we will not approve this PR. The main reason is to avoid having yet another testing infrastructure to maintain. We hope we can enable these same tests soon enough on Testing Farm.

@jpodivin
Copy link
Contributor Author

jpodivin commented Dec 4, 2025

I understand. Closing PR.

@jpodivin jpodivin closed this Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: new

Development

Successfully merging this pull request may close these issues.

4 participants