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

Remove test summary reports #5126

Merged
merged 2 commits into from
Jan 21, 2024

Conversation

albertteoh
Copy link
Contributor

Which problem is this PR solving?

Description of the changes

  • We had a go at using a test summary reporter github action, but it appeared to lack sufficient support for "unexpected" test failures such as panics, while also making it harder to manually inspect test log output.
  • The original log output still satisfies our requirements, as it is still easy to search through to identify the failing test.

Checklist

Signed-off-by: Albert Teoh <albert@packsmith.io>
@albertteoh albertteoh requested a review from a team as a code owner January 21, 2024 00:41
@albertteoh albertteoh added the changelog:ci Change related to continuous integration / testing label Jan 21, 2024
Copy link

codecov bot commented Jan 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (adbdb2d) 95.59% compared to head (5675e49) 95.60%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5126      +/-   ##
==========================================
+ Coverage   95.59%   95.60%   +0.01%     
==========================================
  Files         322      322              
  Lines       18454    18454              
==========================================
+ Hits        17641    17643       +2     
+ Misses        653      651       -2     
  Partials      160      160              
Flag Coverage Δ
cassandra-3.x 25.58% <ø> (ø)
cassandra-4.x 25.58% <ø> (ø)
elasticsearch-5.x 19.87% <ø> (ø)
elasticsearch-6.x 19.87% <ø> (+0.01%) ⬆️
elasticsearch-7.x 19.99% <ø> (ø)
elasticsearch-8.x 20.09% <ø> (ø)
grpc-badger 19.51% <ø> (ø)
kafka 14.09% <ø> (ø)
opensearch-1.x 19.99% <ø> (ø)
opensearch-2.x 19.99% <ø> (-0.02%) ⬇️
unittests 93.29% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Makefile Outdated
@@ -455,10 +454,6 @@ install-ci: install-test-tools install-build-tools
test-ci: GOTEST := $(GOTEST_QUIET) -json
Copy link
Member

Choose a reason for hiding this comment

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

do we still need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, good catch; thanks! Removed -json flag in: 5675e49

Signed-off-by: Albert Teoh <albert@packsmith.io>
@albertteoh
Copy link
Contributor Author

Note the test is failing on "Download and extract artifacts" but this should no longer exist once this PR is merged into main.

@yurishkuro yurishkuro merged commit 6662e1c into jaegertracing:main Jan 21, 2024
37 checks passed
@yurishkuro
Copy link
Member

Thanks!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:ci Change related to continuous integration / testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Test reports don't seem to work
2 participants