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 emission of TestSubtaskEvents #5816

Merged

Conversation

MarlonGamez
Copy link
Contributor

@MarlonGamez MarlonGamez commented May 11, 2021

Fixes: #5784
Related: #5423

Description
Adds emission of TestSubtaskEvents when tests start, finish, or fail

User facing changes (remove if N/A)
Users can now see events that correlate to start, failure, and finish of tests in skaffold

Follow-up Work (remove if N/A)
Put more detail in these events. The initial definition doesn't have much meta info about the test running, but I'd like to update it with more information.

@MarlonGamez MarlonGamez requested a review from a team as a code owner May 11, 2021 19:43
@MarlonGamez MarlonGamez requested a review from nkubala May 11, 2021 19:43
@google-cla google-cla bot added the cla: yes label May 11, 2021
@codecov
Copy link

codecov bot commented May 11, 2021

Codecov Report

Merging #5816 (f768f66) into master (8efc9ef) will increase coverage by 0.04%.
The diff coverage is 85.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5816      +/-   ##
==========================================
+ Coverage   71.00%   71.05%   +0.04%     
==========================================
  Files         438      440       +2     
  Lines       16497    16532      +35     
==========================================
+ Hits        11714    11746      +32     
- Misses       3921     3923       +2     
- Partials      862      863       +1     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/apply.go 25.00% <0.00%> (ø)
cmd/skaffold/app/cmd/delete.go 57.14% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go 56.52% <0.00%> (ø)
cmd/skaffold/app/cmd/diagnose.go 64.51% <ø> (ø)
cmd/skaffold/app/cmd/filter.go 25.58% <0.00%> (ø)
cmd/skaffold/app/cmd/find_configs.go 48.48% <0.00%> (ø)
cmd/skaffold/app/cmd/generate_pipeline.go 61.53% <0.00%> (ø)
cmd/skaffold/app/cmd/render.go 40.74% <0.00%> (ø)
cmd/skaffold/app/cmd/runner.go 58.49% <0.00%> (ø)
cmd/skaffold/app/cmd/test.go 46.66% <0.00%> (ø)
... and 137 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb9c352...f768f66. Read the comment docs.

@@ -370,6 +370,11 @@ func (ev *eventHandler) handleExec(event *proto.Event) {
ev.stateLock.Lock()
ev.state.BuildState.Artifacts[be.Artifact] = be.Status
ev.stateLock.Unlock()
case *proto.Event_TestEvent:
te := e.TestEvent
Copy link
Contributor

Choose a reason for hiding this comment

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

why we need these intermediate vars?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't think we do, this is mostly just a result of me copying from the previous event API 😅 I can make a follow up PR to clean this up a bit!

@MarlonGamez MarlonGamez merged commit 81f3d92 into GoogleContainerTools:master May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have skaffold emit test subtask protos
2 participants