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

Run integration tests only for master branch but keep copr build for all #1831

Closed

Conversation

kontura
Copy link
Contributor

@kontura kontura commented Jun 15, 2022

Running tests for other branches (branches that maintain older rhel and
fedora versions) agains master branch of CI doesn't make much sense and
is usually broken.

We want to keep running copr builds to make sure the package builds.
The ansible tests are testing more basic functionallity and should still
work for dnf.

Running tests for other branches (branches that maintain older rhel and
fedora versions) agains master branch of CI doesn't make much sense and
is usually broken.

We want to keep running copr builds to make sure the package builds.
The ansible tests are testing more basic functionallity and should still
work for dnf.
It doesn't contain just the token but in fact the whole API config
from: https://copr.fedorainfracloud.org/api/
@kontura
Copy link
Contributor Author

kontura commented Jun 16, 2022

I am also considering adding the if: to the ci-dnf-stack integration-tests action which would be better because it would not be duplicated across all repos that use it. However not all the branches are called master - libdnf has dnf-4-master which would complicate the condition and also if the check is in integration-tests action the action actually runs and shows up as successful, example PR: kontura#5, but when approach of this PR is used it is skipped: kontura#7

If we choose to do it like this and I will be creating PRs in all of the components I would also like to rename COPR_API_TOKEN to COPR_API_CONFIG (my second commit). I have got stuck on it twice already while setting up my own workflows and actions to test the CI setup thinking its just the token and then awkwardly debugging it.

@lukash
Copy link
Contributor

lukash commented Jun 16, 2022

I am also considering adding the if: to the ci-dnf-stack integration-tests action which would be better because it would not be duplicated across all repos that use it. However not all the branches are called master - libdnf has dnf-4-master which would complicate the condition and also if the check is in integration-tests action the action actually runs and shows up as successful, example PR: kontura#5, but when approach of this PR is used it is skipped: kontura#7

I think we definitely want to show the CI as skipped and not passing for this case.

If we choose to do it like this and I will be creating PRs in all of the components I would also like to rename COPR_API_TOKEN to COPR_API_CONFIG (my second commit). I have got stuck on it twice already while setting up my own workflows and actions to test the CI setup thinking its just the token and then awkwardly debugging it.

Ok, I understand the confusion. I guess if you name it config then it's then not very clear it does contain authentication credentials. but I don't mind either name...

I'd like to discuss this a bit before merging though.

@kontura
Copy link
Contributor Author

kontura commented Jun 20, 2022

Ok, I understand the confusion. I guess if you name it config then it's then not very clear it does contain authentication credentials. but I don't mind either name...

I think we can go as descriptive as we want here, even something like: COPR_API_CONFIG_WITH_CRENDETIALS.

@kontura
Copy link
Contributor Author

kontura commented Jun 20, 2022

Closing since we have decided that ultimately its not worth it.

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

Successfully merging this pull request may close these issues.

2 participants