Skip to content

Conversation

@DrJosh9000
Copy link
Contributor

@DrJosh9000 DrJosh9000 commented Dec 3, 2025

Description

Add ability to fetch job logs to the E2E test helpers, and show that it works in the basic E2E test.

Context

https://linear.app/buildkite/issue/PB-990/add-mechanism-to-read-job-logs

Changes

Per Description.

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go tool gofumpt -extra -w .)
  • It works

Disclosures / Credits

Some bits copied from https://github.com/buildkite/agent-stack-k8s.

@DrJosh9000 DrJosh9000 requested a review from a team December 3, 2025 05:13
Copy link
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

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

Looks good. I have a suggestion. It's not strictly blocking so I will approve it.

Comment on lines 31 to 33
if err != nil {
t.Errorf("tc.fetchLogs() error = %v", err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this error handling and the waitForBuild error handling can all happen in the callee side I think, I don't think they will ever be useful on caller side?

Since we have a t *testing.T available, we can effectively "throw" error from callee side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's probably true. I'm not sure we'll need a case where they can fail, so I'll move that.

@DrJosh9000 DrJosh9000 force-pushed the pb-990-add-mechanism-to-read-job-logs branch 3 times, most recently from abe178a to b585aa2 Compare December 3, 2025 05:47
@DrJosh9000 DrJosh9000 force-pushed the pb-990-add-mechanism-to-read-job-logs branch from b585aa2 to 3761685 Compare December 3, 2025 05:51
@DrJosh9000 DrJosh9000 enabled auto-merge December 3, 2025 05:54
@DrJosh9000 DrJosh9000 merged commit 1c367d1 into main Dec 3, 2025
2 checks passed
@DrJosh9000 DrJosh9000 deleted the pb-990-add-mechanism-to-read-job-logs branch December 3, 2025 05:56
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.

3 participants