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

Do not show build status in manual test checks #2375

Conversation

majamassarini
Copy link
Member

@majamassarini majamassarini commented Mar 18, 2024

Unless there are failures or errors.

This can be improved having a real dependency
between tests and builds.

Showing a neutral state for a successful build
message is not working on gitlab and either on
github is not working always
(my test falls back on commit statuses and
they don't have a neutral state).

Fixes #2147

RELEASE NOTES BEGIN

Packit no longer shows status checks for not yet triggered manual tests.

RELEASE NOTES END

Copy link
Contributor

@@ -652,6 +652,9 @@ def report_status_to_all_test_jobs(
for test_job in self.job_tests_all:
if test_job.skip_build:
continue
elif test_job.manual_trigger:
if state not in (BaseCommitStatus.error, BaseCommitStatus.failure):
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought that showing errors can make it easier for a user to understand why tests are not running, but then I realized that when the copr build will work the failed manual test check will not be overwritten. And this is misleading. So it is probably better never to show these check statuses at all. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I would rather be consistent and really hide all the statuses

Copy link
Contributor

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

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

otherwise looks good 👍

@@ -652,6 +652,9 @@ def report_status_to_all_test_jobs(
for test_job in self.job_tests_all:
if test_job.skip_build:
continue
elif test_job.manual_trigger:
if state not in (BaseCommitStatus.error, BaseCommitStatus.failure):
Copy link
Member

Choose a reason for hiding this comment

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

I would rather be consistent and really hide all the statuses

Copy link
Contributor

This can be improved having a real dependency
between tests and builds.

Showing a neutral state for a successful build
message is not working on gitlab and even on
github is not always working
(my test falls back on commit statuses and
they don't have a neutral state).

I can not show only errors related with build
statuses because they will be not cleaned up by
successful statuses later and this is misleading.

We can just hide them all, at least until we
have real dependencies.
Copy link
Contributor

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

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

thanks!

@majamassarini majamassarini added the mergeit When set, zuul wil gate and merge the PR. label Mar 21, 2024
Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/98fab7b3a05e42bdad0c6e83365ec49c

✔️ pre-commit SUCCESS in 2m 18s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 0a85e3c into packit:main Mar 21, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFE] Do not show queued jobs
2 participants