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

promql needlessly imports "testing" which adds a bunch of needless flags #3915

Closed
Dieterbe opened this Issue Mar 6, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@Dieterbe
Copy link
Contributor

Dieterbe commented Mar 6, 2018

github.com/prometheus/prometheus/promql imports github.com/prometheus/prometheus/util/testutil which imports "testing".

the result is that any tool (3rd party) which imports github.com/prometheus/prometheus/promql imports testing, which if said tool uses flags, then a bunch of flags are added automatically by the testing package:

  -test.bench regexp
    	run only benchmarks matching regexp
  -test.benchmem
    	print memory allocations for benchmarks
  -test.benchtime d
    	run each benchmark for duration d (default 1s)
  -test.blockprofile file
    	write a goroutine blocking profile to file
  -test.blockprofilerate rate
    	set blocking profile rate (see runtime.SetBlockProfileRate) (default 1)
  -test.count n
    	run tests and benchmarks n times (default 1)
  -test.coverprofile file
    	write a coverage profile to file
  -test.cpu list
    	comma-separated list of cpu counts to run each test with
  -test.cpuprofile file
    	write a cpu profile to file
  -test.failfast
    	do not start new tests after the first test failure
  -test.list regexp
    	list tests, examples, and benchmarks matching regexp then exit
  -test.memprofile file
    	write a memory profile to file
  -test.memprofilerate rate
    	set memory profiling rate (see runtime.MemProfileRate)
  -test.mutexprofile string
    	write a mutex contention profile to the named file after execution
  -test.mutexprofilefraction int
    	if >= 0, calls runtime.SetMutexProfileFraction() (default 1)
  -test.outputdir dir
    	write profiles to dir
  -test.parallel n
    	run at most n tests in parallel (default 8)
  -test.run regexp
    	run only tests and examples matching regexp
  -test.short
    	run smaller test suite to save time
  -test.testlogfile file
    	write test action log to file (for use only by cmd/go)
  -test.timeout d
    	panic test binary after duration d (default 0, timeout disabled)
  -test.trace file
    	write an execution trace to file
  -test.v
    	verbose: print additional output

these flags are irrelevant and thus confusing. they pollute help/usage messages for tools that don't invoke any testing logic.
as far as I know, only test code should actually import "testing"; not regular application code or libraries.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented Mar 8, 2018

This came up in the past. Remvoing the dependency was tricky for various reasons.
We concluded that the right answer is to not use the global flag set.

@Dieterbe

This comment has been minimized.

Copy link
Contributor Author

Dieterbe commented Mar 8, 2018

so, if the testing flags are not desired, apps should not define any global flags then?
That would be a bit painful for me, maybe i'll try to remove the testing flags out of the global flagset instead.

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented Mar 8, 2018

so, if the testing flags are not desired, apps should not define any global flags then?

Yes. That's generally better in every way.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 20, 2018

Do you have a proposal for disentangling this?

@Dieterbe

This comment has been minimized.

Copy link
Contributor Author

Dieterbe commented Mar 20, 2018

me? no. realistically i won't have time to work on this. (if no one has, feel free to close this)

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Mar 20, 2018

We can put it down as something to consider as we develop a promql testing tool for users, but beyond that I don't think anyone is prioritising this.

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 22, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 22, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.