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

Use env variable for unit test verbose mode #2572

Conversation

@amitkrout
Copy link
Collaborator

amitkrout commented Feb 6, 2020

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind feature

What does does this PR do / why we need it:
Enabling verbose mode for unit test run. Helps to debugging the unit test failure
Which issue(s) this PR fixes:

Fixes #? NA

How to test changes / Special notes to the reviewer:

UNIT_TEST_VERBOSE_MODE=-v make test

@amitkrout

This comment has been minimized.

Copy link
Collaborator Author

amitkrout commented Feb 7, 2020

/test v4.1-integration-e2e-benchmark

@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 7, 2020

Codecov Report

Merging #2572 into master will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2572     +/-   ##
=========================================
- Coverage   42.77%   42.67%   -0.1%     
=========================================
  Files          74       74             
  Lines        7089     7089             
=========================================
- Hits         3032     3025      -7     
- Misses       3768     3772      +4     
- Partials      289      292      +3
Impacted Files Coverage Δ
pkg/component/watch.go 68% <0%> (-4.67%) ⬇️

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 e21d6c6...743bc8f. Read the comment docs.

Makefile Outdated
# Env variable UNIT_TEST_VERBOSE_MODE is used to get control over enabling go test
# verbose mode for unit test run. By default go test verbosity is not enabled.
# To enable verbosity export or set env UNIT_TEST_VERBOSE_MODE like "UNIT_TEST_VERBOSE_MODE=-v"
UNIT_TEST_VERBOSE_MODE ?=

This comment has been minimized.

Copy link
@girishramnani

girishramnani Feb 10, 2020

Collaborator

it should either be UNIT_TEST_VERBOSE_MODE=true or UNIT_TEST_ARGS=-v

This comment has been minimized.

Copy link
@amitkrout

amitkrout Feb 11, 2020

Author Collaborator

@girishramnani

UNIT_TEST_VERBOSE_MODE=true

Looks good, but it needs few more unnecessary lines in make file to implement this. Unnecessary because we have already a -v flag which enables unit test verbose in golang. For example go test -v. Here just need to use the -v flag through an env var.

UNIT_TEST_ARGS=-v

Sounds good, but it is not specific around which args it holds. So imo UNIT_TEST_VERBOSE_MODE ?= is not a bad choice ;)

This comment has been minimized.

Copy link
@kadel

kadel Feb 11, 2020

Member

UNIT_TEST_ARGS=-v

Sounds good, but it is not specific around which args it holds. So imo UNIT_TEST_VERBOSE_MODE ?= is not a bad choice ;)

That is exactly the intention. This variable can hold any flags for go test not just -v. So something like UNIT_TEST_ARGS makes more sense.
If you want to limit it to just verbose mode than you should make it UNIT_TEST_VERBOSE_MODE=true and add those few additional lines.

Btw GINKGO_VERBOSE_MODE has the same issue ;-)

This comment has been minimized.

Copy link
@girishramnani

girishramnani Feb 11, 2020

Collaborator

couldn't have said it better myself 👍 @kadel

This comment has been minimized.

Copy link
@amitkrout

amitkrout Feb 12, 2020

Author Collaborator

@kadel Awesome explanation. Review comments are addressed.

@girishramnani

This comment has been minimized.

Copy link
Collaborator

girishramnani commented Feb 10, 2020

I would suggest that we use either UNIT_TEST_VERBOSE_MODE=true or UNIT_TEST_ARGS=-v because UNIT_TEST_VERBOSE_MODE=-v feels a bit confusing

@amitkrout

This comment has been minimized.

Copy link
Collaborator Author

amitkrout commented Feb 11, 2020

I would suggest that we use either UNIT_TEST_VERBOSE_MODE=true or UNIT_TEST_ARGS=-v because UNIT_TEST_VERBOSE_MODE=-v feels a bit confusing

@kadel @girishramnani https://github.com/openshift/odo/pull/2572/files#r377582865

@amitkrout amitkrout force-pushed the amitkrout:useEnvVarForUnitTestVervoseLog branch from 0475fd3 to ede8a72 Feb 12, 2020
@openshift-ci-robot openshift-ci-robot added size/S and removed size/XS labels Feb 12, 2020
@amitkrout amitkrout force-pushed the amitkrout:useEnvVarForUnitTestVervoseLog branch from 9a05306 to ba62c84 Feb 12, 2020
@amitkrout amitkrout force-pushed the amitkrout:useEnvVarForUnitTestVervoseLog branch from ba62c84 to 743bc8f Feb 12, 2020
@kadel

This comment has been minimized.

Copy link
Member

kadel commented Feb 12, 2020

/approve

@openshift-ci-robot

This comment has been minimized.

Copy link

openshift-ci-robot commented Feb 12, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@amitkrout

This comment has been minimized.

Copy link
Collaborator Author

amitkrout commented Feb 13, 2020

/test v4.3-integration-e2e-benchmark

@prietyc123

This comment has been minimized.

Copy link
Collaborator

prietyc123 commented Feb 13, 2020

Looks good to me
/lgtm

@openshift-merge-robot openshift-merge-robot merged commit bba04d2 into openshift:master Feb 13, 2020
6 checks passed
6 checks passed
Travis CI - Pull Request Build Passed
Details
ci/prow/unit Job succeeded.
Details
ci/prow/v4.1-integration-e2e-benchmark Job succeeded.
Details
ci/prow/v4.2-integration-e2e-benchmark Job succeeded.
Details
ci/prow/v4.3-integration-e2e-benchmark Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.