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

Differentiate between "empty result of targets_override" and "don't use targets_override" #1519

Closed
2 tasks
nikromen opened this issue May 24, 2022 · 2 comments · Fixed by #2665
Closed
2 tasks
Assignees
Labels
area/testing-farm Related to Testing Farm integration. complexity/single-task Regular task, should be done within days. gain/low This doesn't bring that much value to users. impact/high This issue impacts multiple/lot of users. kind/bug Something isn't working. workaround-exists There is an existing workaround that can be used in the meantime of implementing the issue.

Comments

@nikromen
Copy link
Member

Surprise :) new edgecase bug discovered for /packit retest-failed 🐛
/packit retest-failed restarts everything on these circumstances:

  • both - tests jobs and copr-build jobs in packit.yaml must be enabled
  • some RPM build has to fail -> test for this chroot is not started
  • on /packit retest-failed -> every test is restarted

This is caused because of previously failed build: build fails -> test is not started (but ends up with a failure and with message that RPM build failed so we don't run test). That means there isn't any failed test in the database so set of chroots to override of /packit retest-failed is (correctly) {}. And here comes the issue - when targets_override are {} or None we build/test for every chroot.

Correctly, the user should call /packit rebuild-failed (but a mess happens when he does it wrong way).
You can try/see this behavior here
(This behavior obviously reveals when user calls rebuild/retest on fully passed builds/tests)

Possible solution: we should differentiate between {} and None in targets_override. If result is {} - we should "override" targets with "nothing" (or just ignore it).

And we should copy the behavior of /packit test for this - when RPM build fails and user triggers /packit test again, we notify the user with The latest build was not successful, not running tests for it. . We should respond to user the same/similar note when he tries to retest-failed, but there was previously failed RPM build.

TODO:

  • ignore building/testing when result of targets_override is empty set.
  • when user does retest-failed on tests where RPM build failed - provide a note that latest build was not successfull
@stale
Copy link

stale bot commented Sep 21, 2022

This issue has been marked as stale because it hasn't seen any
activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed
by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! 🦄 🚀 🤖

(Note: issues labeled with pinned or EPIC are
never marked as stale.)

@stale stale bot added the stale Is the issue still valid? label Sep 21, 2022
@TomasTomecek TomasTomecek removed the stale Is the issue still valid? label Dec 6, 2022
@jpopelka jpopelka changed the title Differentiate between "empty result of targets_override" and "don't use tagrets_override" Differentiate between "empty result of targets_override" and "don't use targets_override" Dec 6, 2022
@lbarcziova
Copy link
Member

Verified this use case still doesn't work as expected in packit/hello-world#1432: every test was restarted, but for the one with the missing successful build, the status was immediately The latest build was not successful, not running tests for it.

@lbarcziova lbarcziova added kind/bug Something isn't working. area/testing-farm Related to Testing Farm integration. workaround-exists There is an existing workaround that can be used in the meantime of implementing the issue. impact/high This issue impacts multiple/lot of users. gain/low This doesn't bring that much value to users. complexity/single-task Regular task, should be done within days. labels May 11, 2023
@lbarcziova lbarcziova removed their assignment Sep 8, 2023
@lachmanfrantisek lachmanfrantisek moved this from backlog to priority-backlog in Packit Kanban Board Jan 25, 2024
@majamassarini majamassarini moved this from priority-backlog to in-progress in Packit Kanban Board Nov 18, 2024
@majamassarini majamassarini self-assigned this Nov 18, 2024
@github-project-automation github-project-automation bot moved this from in-progress to done in Packit Kanban Board Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing-farm Related to Testing Farm integration. complexity/single-task Regular task, should be done within days. gain/low This doesn't bring that much value to users. impact/high This issue impacts multiple/lot of users. kind/bug Something isn't working. workaround-exists There is an existing workaround that can be used in the meantime of implementing the issue.
Projects
Archived in project
4 participants