Skip to content

Add verbosity flag to go tests on travis#3548

Merged
tejal29 merged 4 commits intomasterfrom
verbose-test-on-travis
Jan 22, 2020
Merged

Add verbosity flag to go tests on travis#3548
tejal29 merged 4 commits intomasterfrom
verbose-test-on-travis

Conversation

@loosebazooka
Copy link
Copy Markdown
Member

@loosebazooka loosebazooka commented Jan 21, 2020

No description provided.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 21, 2020 •
edited
Loading

Codecov Report

Merging #3548 into master will increase coverage by 0.19%.
The diff coverage is n/a.

Impacted Files Coverage Δ
pkg/skaffold/initializer/config.go 100% <0%> (ø) ⬆️
pkg/skaffold/initializer/init.go 0% <0%> (ø) ⬆️
pkg/skaffold/initializer/prompt.go 0% <0%> (ø) ⬆️
pkg/skaffold/initializer/util.go
pkg/skaffold/schema/defaults/defaults.go 94.38% <0%> (+0.12%) ⬆️
pkg/skaffold/initializer/builders.go 94.2% <0%> (+0.76%) ⬆️
pkg/skaffold/initializer/analyze.go 87.23% <0%> (+4.88%) ⬆️

@loosebazooka loosebazooka added the kokoro:run runs the kokoro jobs on a PR label Jan 21, 2020
@nkubala nkubala added kokoro:run runs the kokoro jobs on a PR and removed kokoro:run runs the kokoro jobs on a PR labels Jan 21, 2020
@tejal29
Copy link
Copy Markdown
Contributor

tejal29 commented Jan 21, 2020

The flag only got applied to windows tests. Is there a reason for adding the -v flag ?

@loosebazooka
Copy link
Copy Markdown
Member Author

loosebazooka commented Jan 21, 2020 •
edited
Loading

AFAIK, the -v flag prints out the reason the test failed (rather than just saying the test failed), this is nice for people who cannot mimic the test environment easy (ex: I do not have a mac to test mac failures on).

It should apply to all test environments.

@loosebazooka loosebazooka removed the kokoro:run runs the kokoro jobs on a PR label Jan 21, 2020
@loosebazooka
Add -v for make quicktest
77d06fc 
change commit message to trigger build
@loosebazooka loosebazooka force-pushed the verbose-test-on-travis branch from 460b557 to 77d06fc Compare January 21, 2020 22:47
@tejal29 tejal29 self-assigned this Jan 21, 2020
@tejal29
Copy link
Copy Markdown
Contributor

tejal29 commented Jan 21, 2020

AFAIK, the -v flag prints out the reason the test failed (rather than just saying the test failed), this is nice for people who cannot mimic the test environment easy (ex: I do not have a mac to test mac failures on).

It should apply to all test environments.

Ahh got it. Thanks!

@tejal29
Copy link
Copy Markdown
Contributor

tejal29 commented Jan 21, 2020

The linux job still does not have -v flag
https://travis-ci.com/GoogleContainerTools/skaffold/jobs/278134461#L236
Rest all (windows, OSX and integration test) have -v flag.

@tejal29
Copy link
Copy Markdown
Contributor

tejal29 commented Jan 21, 2020

Can you also add -v to coverage target used for linux profile.
https://github.com/GoogleContainerTools/skaffold/blob/master/Makefile#L111

tejal29
tejal29 suggested changes Jan 21, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tejal29 tejal29 left a comment

Choose a reason for hiding this comment

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

Can you also add -v to coverage target used for linux profile.
https://github.com/GoogleContainerTools/skaffold/blob/master/Makefile#L111

@loosebazooka
Copy link
Copy Markdown
Member Author

loosebazooka commented Jan 21, 2020 •
edited
Loading

Do we want it on coverage, isn't that just to send to the coverage service? I put it on https://github.com/GoogleContainerTools/skaffold/blob/master/Makefile#L106 instead.

@loosebazooka loosebazooka requested a review from tejal29 January 21, 2020 23:31
@loosebazooka
Copy link
Copy Markdown
Member Author

loosebazooka commented Jan 21, 2020

oh no you're right, needs to go on coverage.

@tejal29 tejal29 merged commit b5fd53a into master Jan 22, 2020
@loosebazooka loosebazooka mentioned this pull request Jan 23, 2020
Merged
@briandealwis briandealwis deleted the verbose-test-on-travis branch April 14, 2020 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@balopat balopat Awaiting requested review from balopat

@dgageot dgageot Awaiting requested review from dgageot

@nkubala nkubala Awaiting requested review from nkubala

1 more reviewer

@tejal29 tejal29 tejal29 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tejal29 tejal29

Labels

cla: yes size/XS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@loosebazooka @tejal29 @googlebot @nkubala