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

Add support for issue metadata and dynamic summary #234

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

jamacku
Copy link
Member

@jamacku jamacku commented Sep 20, 2024

It replaces the old comment logic with the new one that uses issue metadata
to story helper data. It allows us to create only one comment with a summary
of all tests and their results, which is dynamically updated.

Also, enable issue comments for our Testing Farm GitHub Action repository.


Replaces:

TODO:

  • Show some pictures
  • Handle cancel events

@jamacku jamacku marked this pull request as draft September 20, 2024 15:10
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

Attention: Patch coverage is 88.81119% with 32 lines in your changes missing coverage. Please review.

Project coverage is 91.47%. Comparing base (d2b99fd) to head (8066095).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/pull-request.ts 46.42% 30 Missing ⚠️
src/metadata.ts 98.33% 1 Missing ⚠️
src/summary.ts 99.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
- Coverage   93.37%   91.47%   -1.91%     
==========================================
  Files          11       14       +3     
  Lines         604      856     +252     
  Branches      114      156      +42     
==========================================
+ Hits          564      783     +219     
- Misses         40       73      +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jamacku

This comment was marked as outdated.

1 similar comment
@jamacku

This comment was marked as outdated.

@jamacku

This comment was marked as outdated.

@jamacku

This comment was marked as outdated.

1 similar comment
@jamacku

This comment was marked as outdated.

This comment was marked as duplicate.

@jamacku

This comment was marked as outdated.

@sclorg sclorg deleted a comment from github-actions bot Sep 23, 2024
@sclorg sclorg deleted a comment from github-actions bot Sep 23, 2024
@sclorg sclorg deleted a comment from github-actions bot Sep 23, 2024
@sclorg sclorg deleted a comment from github-actions bot Sep 23, 2024
@sclorg sclorg deleted a comment from github-actions bot Sep 23, 2024
@sclorg sclorg deleted a comment from github-actions bot Sep 23, 2024
@sclorg sclorg deleted a comment from github-actions bot Sep 23, 2024
@jamacku

This comment was marked as outdated.

4 similar comments
@jamacku

This comment was marked as outdated.

@jamacku

This comment was marked as outdated.

@jamacku

This comment was marked as outdated.

@jamacku

This comment was marked as outdated.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

This comment was marked as duplicate.

@jamacku

This comment was marked as outdated.

Copy link

github-actions bot commented Sep 23, 2024

Testing Farm results

namecomposearchstatustimelogs
Timeout testFedora-latestx86_64✅ passed8min 18stest pipeline
Smoke testFedora-latestx86_64✅ passed3min 20stest pipeline
Secrets testFedora-latestx86_64✅ passed3min 3stest pipeline
Variables testFedora-latestx86_64✅ passed3min 52stest pipeline

@jamacku

This comment was marked as outdated.

1 similar comment
@jamacku

This comment was marked as outdated.

@jamacku

This comment was marked as outdated.

2 similar comments
@jamacku

This comment was marked as outdated.

@jamacku

This comment was marked as outdated.

@jamacku jamacku force-pushed the summary branch 2 times, most recently from 371d626 to cb0590d Compare September 24, 2024 08:55
@jamacku jamacku changed the title feat: add support for issue metadata and dynamic summary Add support for issue metadata and dynamic summary Sep 24, 2024
@jamacku jamacku marked this pull request as ready for review September 24, 2024 08:56
It replaces the old comment logic with the new one that uses issue metadata
to story helper data. It allows us to create only one comment with a summary
of all tests and their results which is dynamically updated.

Also enable issue comments for our Testing Farm GitHub Action repository.
Copy link
Member

@phracek phracek left a comment

Choose a reason for hiding this comment

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

I don't know now if this change is braking, but I have one proposal. I guess 'pull_request_status_name' needs to be dependent on Summary.
What about if 'pull_request_status_name' is not defined?

Copy link
Member

@phracek phracek left a comment

Choose a reason for hiding this comment

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

But this is amazing work. Thanks for moving TFaGA to be more effective and usable for users.

@jamacku
Copy link
Member Author

jamacku commented Sep 24, 2024

I don't know now if this change is braking, but I have one proposal.

I set it as breaking because we are changing the content and behavior of the comment. Previously, we created a new comment for each run of the Action (a new notification was sent). Now, we create only one comment, which we update, so only one notification is sent.

I guess 'pull_request_status_name' needs to be dependent on Summary. What about if 'pull_request_status_name' is not defined?

pull_request_status_name has a default value of Fedora. But I think everyone is using statuses and changing/setting the pull_request_status_name input to some value.

Copy link
Member

@phracek phracek 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

@phracek phracek merged commit a0f6ca1 into sclorg:main Sep 24, 2024
5 of 7 checks passed
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.

Add possibility to create comment with results
2 participants