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

Various improvements to testing #421

Merged
merged 10 commits into from
Jan 9, 2025
Merged

Conversation

cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Oct 30, 2024

  • Add some missing t.Parallal() and startTestSpan calls
  • Fix test log output to print line by line instead of per read
  • Use the same cache keys for the main Dockerfile and the test fixtures
  • Add test stats to GHA summary
  • Get tracing reports from integration tests
  • Remove GH cache since this wasn't really giving us anything, and maybe was even slowing things down
  • ci: Fix issue reporting failures from test panic
  • ci: Split integration tests per distro into separate jobs

Supersedes #482
Fixes #476

@cpuguy83 cpuguy83 force-pushed the improve_ci_times branch 30 times, most recently from 4754f3b to 1a0e72b Compare November 4, 2024 16:08
@cpuguy83 cpuguy83 force-pushed the improve_ci_times branch 7 times, most recently from 18661ea to f62af9d Compare December 23, 2024 18:35
@cpuguy83 cpuguy83 force-pushed the improve_ci_times branch 2 times, most recently from 9ad6394 to c2f33b8 Compare December 23, 2024 19:04
Before this was calling `t.Log` for every read from the log file.
What we really want is to to call `t.Log` for every line since a `Read`
may return a partial line which will look weird in the output and be
difficult to read.

This also changes things to always write the build logs to the test log
instead of just on failure so its easier to see what's happening even on
a successful test run.

Signed-off-by: Brian Goff <[email protected]>
This makes sure we use the same cache keys for both the main Dockerfile
and the test fixtures so that we can avoid needing to pull down a bunch
of dependencies that we've already pulled down before.

Signed-off-by: Brian Goff <[email protected]>
@cpuguy83
Copy link
Member Author

cpuguy83 commented Jan 8, 2025

The test failure is actually on main... I'm bisecting this now.

@cpuguy83
Copy link
Member Author

cpuguy83 commented Jan 8, 2025

Pushed commit to fix.

Copy link
Contributor

@adamperlin adamperlin left a comment

Choose a reason for hiding this comment

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

Just some minor comments

cmd/test2json2gha/event.go Outdated Show resolved Hide resolved
cmd/test2json2gha/main.go Show resolved Hide resolved
This doesn't end up saving us any time and potentially slows down a test
run even so we can just remove it.
This was ignoring all non-test events, unfortunately this means if there
is a panic or a test timeout we may not detect the failure.

Signed-off-by: Brian Goff <[email protected]>
These should not be processed since any variable substitution should be
handled by env.

Also to note: This use of arg expansion was causing tests to fail,
though we didn't see it due to some issues with CI infra.

Signed-off-by: Brian Goff <[email protected]>
@cpuguy83 cpuguy83 merged commit 3c4c901 into Azure:main Jan 9, 2025
18 checks passed
@cpuguy83 cpuguy83 deleted the improve_ci_times branch January 9, 2025 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Integration tests possible not reporting failures
2 participants