Skip to content

Conversation

@ivanvc
Copy link
Member

@ivanvc ivanvc commented Oct 30, 2025

Remove the custom environment variable GOLANG_TEST_SHORT.

It is currently used by Main tests (TestMain), as we can't skip tests because testing.M doesn't implement testing.TB. It is possible to do a clean os.Exit(0) (current behavior), but calling testing.Short() fails because this function expects flags to be parsed before.

So, it is possible to remove the custom behavior (GOLANG_TEST_SHORT) by parsing flags (if required) before calling testing.Short(), then immediately exit if the result is true (-short flag is set).

I believe the only place where this environment variable is used is by tests/integration/clientv3/concurrency (and tests/integration/clientv3/examples). Running with the new approach using only -short, works as expected:

$ pwd
[...]/etcd/tests/integration/clientv3/concurrency
$ go test . -count 1 -short -v
2025/10/30 10:20:49 Skipping: the tests require real cluster
ok      go.etcd.io/etcd/tests/v3/integration/clientv3/concurrency       0.012s
$ go test . -count 1
ok      go.etcd.io/etcd/tests/v3/integration/clientv3/concurrency       3.172s

Omitting the verbose output, but focus on time spent and the new log line when short is set.

Part of #18409, as I found out about this while working on updating/simplifying test targets.

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

Remove the custom environment variable GOLANG_TEST_SHORT.

It is currently used by Main tests (TestMain), as we can't skip tests
because testing.M doesn't implement testing.TB. It is possible to do a
clean os.Exit(0) (current behavior), but calling testing.Short() fails
because this function expects flags to be parsed before.

So, it is possible to remove the custom behavior (GOLANG_TEST_SHORT) by
parsing flags (if required) before calling testing.Short(), then
immediately exit if the result is true (-short flag is set).

Signed-off-by: Ivan Valdes <[email protected]>
@codecov
Copy link

codecov bot commented Oct 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.19%. Comparing base (29cd6bb) to head (3e57a17).

Additional details and impacted files

see 25 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #20873      +/-   ##
==========================================
+ Coverage   69.13%   69.19%   +0.06%     
==========================================
  Files         422      422              
  Lines       34809    34814       +5     
==========================================
+ Hits        24064    24091      +27     
+ Misses       9349     9326      -23     
- Partials     1396     1397       +1     

Continue to review full report in Codecov by Sentry.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ivanvc
Copy link
Member Author

ivanvc commented Oct 30, 2025

/retest

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

thx for the cleanup.

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, ivanvc

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants