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

BUG: Ginkgo not calculating as expected coverage #1161

Closed
segator opened this issue Mar 9, 2023 · 17 comments
Closed

BUG: Ginkgo not calculating as expected coverage #1161

segator opened this issue Mar 9, 2023 · 17 comments

Comments

@segator
Copy link

segator commented Mar 9, 2023

tested on v.2.9.0

ginkgo -coverpkg=./... -coverprofile=ginko.out --output-dir build/ ./...

go test -coverpkg=./... -coverprofile=build/gotest.out ./...

number of lines in the ginkgo.out 85
number of lines in the gotest.out 773

I tried in specific folders within my project like ./cli where my cobra commands are in place
and same behaviour but at folder level, less lines of test are covered in ginkgo.
After comparing 2 files, I have the feeling that ginkgo only take into account coverage of code of the instructed folder, but this folder is possible is calling to functions in other packages.

@onsi
Copy link
Owner

onsi commented Mar 9, 2023

hey there - this has to do with how Ginkgo switches paths when running multiple packages:

#995

right now one needs to specify all -coverpkg as absolute paths and ./... won't work. it's something i need to fix but haven't been able to prioritize yet.

@segator
Copy link
Author

segator commented Mar 9, 2023

I see, is there any workarround we can set at cli meanwhile?

@onsi
Copy link
Owner

onsi commented Mar 9, 2023

i really should fix it in ginkgo - i'll try to get to it soon. in the mean time if you avoid relative paths things should work. If the module you are testing is called example.com/foo/bar then I think you can do:

ginkgo  -coverpkg=example.com/foo/bar/... -coverprofile=ginko.out --output-dir build/  ./...

alternatively you can pass in a list of file paths. unfortunately it doesn't look like go lets you specify something like -coverpkg=/path/to/src/... but you can do -coverpkg=/path/to/a,/path/to/b

@onsi
Copy link
Owner

onsi commented Mar 12, 2023

this should now be fixed in v2.9.1 - ginkgo -coverpkg=./... -r will now compute coverage for all code by all tests.

please give it a try and let me know if it works for you. then we can close this issue out.

@segator
Copy link
Author

segator commented Mar 14, 2023

Yup Working, Thanks for the fix 🥇

@segator segator closed this as completed Mar 14, 2023
@segator segator reopened this Mar 14, 2023
@segator
Copy link
Author

segator commented Mar 14, 2023

Something is still wrong, I see better coverage than before but I still get less lines of code coverage than using go test directly.

4215 vs 7030

@onsi
Copy link
Owner

onsi commented Mar 14, 2023

odd - can you share a screenshot of all the output when you run go test and then all the output when you run ginkgo? also can you share exactly what you're running on the command line so i can make sure i'm following?

@onsi
Copy link
Owner

onsi commented Mar 14, 2023

when i tested this locally i was getting identical % combined coverages on the command line. do you see a discrepancy there? or is the discrepancy only in the resulting coverprofile file?

if the latter then there may be more work i need to do now when merging the cover profiles

one thing you could try that would help be debug this is to see if you get the same issue when you run -coverpkg=github.com/your/module/... instead of ./...

@segator
Copy link
Author

segator commented Mar 15, 2023

Full go test command

 go test -count=1 -coverpkg=./... -race -short -timeout 600s -coverprofile=build/gotest.out ./...

?       edgeproxy       [no test files]
ok      edgeproxy/cli   1.124s  coverage: 29.3% of statements in ./...
ok      edgeproxy/client/clientauth     0.248s  coverage: 4.2% of statements in ./...
ok      edgeproxy/client/proxy  0.839s  coverage: 10.7% of statements in ./...
?       edgeproxy/config        [no test files]
?       edgeproxy/daemon        [no test files]
?       edgeproxy/dialer        [no test files]
ok      edgeproxy/e2e   40.047s coverage: 62.0% of statements in ./...
?       edgeproxy/env   [no test files]
ok      edgeproxy/helm/test     0.483s  coverage: 2.7% of statements in ./...
?       edgeproxy/log   [no test files]
ok      edgeproxy/monitoring    0.226s  coverage: 7.0% of statements in ./...
?       edgeproxy/monitoring/router     [no test files]
?       edgeproxy/server        [no test files]
ok      edgeproxy/server/auth   1.091s  coverage: 10.7% of statements in ./...
?       edgeproxy/server/handlers       [no test files]
ok      edgeproxy/stream        1.113s  coverage: 2.7% of statements in ./...
?       edgeproxy/test  [no test files]
ok      edgeproxy/transport     0.111s  coverage: 9.7% of statements in ./...
?       edgeproxy/version       [no test files]

Full ginkgo command

ginkgo -r -p --randomize-all --randomize-suites  --race --succinct --keep-going --timeout 600s --output-dir build/ -coverpkg=./... -coverprofile=ginkgo.out ./...

=== RUN   TestJwtAuthenticatorSetSigningKeyPKCS8Key
--- PASS: TestJwtAuthenticatorSetSigningKeyPKCS8Key (0.00s)
=== RUN   TestJwtAuthenticatorSetSigningKeyPKCS1Key
--- PASS: TestJwtAuthenticatorSetSigningKeyPKCS1Key (0.66s)
=== RUN   TestJwtAuthenticatorSetSigningKeyUnsupportedKey
--- PASS: TestJwtAuthenticatorSetSigningKeyUnsupportedKey (0.01s)
=== RUN   TestJwtAuthenticatorSetSigningKeyInvalidKey
--- PASS: TestJwtAuthenticatorSetSigningKeyInvalidKey (0.00s)
PASS
coverage: 14.7% of statements in ./../../...
=== RUN   TestClientMonitoringCliDefaultMetricsPort
--- PASS: TestClientMonitoringCliDefaultMetricsPort (0.62s)
=== RUN   TestClientMonitoringCliCustomMetricsPort
--- PASS: TestClientMonitoringCliCustomMetricsPort (0.51s)
=== RUN   TestServerCli
=== RUN   TestServerCli/Log_formatted_in_JSON
=== RUN   TestServerCli/Detect_running_in_kubernetes
=== RUN   TestServerCli/Detect_running_in_prod_mode
=== RUN   TestServerCli/Verbose_enabled
=== RUN   TestServerCli/Debug_entry_printed
--- PASS: TestServerCli (0.71s)
    --- PASS: TestServerCli/Log_formatted_in_JSON (0.00s)
    --- PASS: TestServerCli/Detect_running_in_kubernetes (0.00s)
    --- PASS: TestServerCli/Detect_running_in_prod_mode (0.00s)
    --- PASS: TestServerCli/Verbose_enabled (0.00s)
    --- PASS: TestServerCli/Debug_entry_printed (0.00s)
PASS
coverage: 29.5% of statements in ./../...
=== RUN   TestClientMetricsServer
--- PASS: TestClientMetricsServer (0.11s)
=== RUN   TestClientMonitoringServerGetMetrics
--- PASS: TestClientMonitoringServerGetMetrics (0.12s)
PASS
coverage: 38.8% of statements in ./../...
=== RUN   TestSpireAuthorization
=== RUN   TestSpireAuthorization/Valid_Certificate_Authorized_User
=== RUN   TestSpireAuthorization/Valid_Certificate_Intermediate_Signed_Authorized_User
=== RUN   TestSpireAuthorization/Valid_Certificate_Intermediate_Signed_Authorized_User/Revoked_Certificate
=== RUN   TestSpireAuthorization/Valid_Certificate_UnAuthorized_User_is_forbidden
=== RUN   TestSpireAuthorization/Valid_Certificate_Not_Spiffe_SNI
=== RUN   TestSpireAuthorization/Certificate_Signed_by_unknown_authority
--- PASS: TestSpireAuthorization (39.46s)
    --- PASS: TestSpireAuthorization/Valid_Certificate_Authorized_User (4.67s)
    --- PASS: TestSpireAuthorization/Valid_Certificate_Intermediate_Signed_Authorized_User (10.27s)
        --- PASS: TestSpireAuthorization/Valid_Certificate_Intermediate_Signed_Authorized_User/Revoked_Certificate (0.06s)
    --- PASS: TestSpireAuthorization/Valid_Certificate_UnAuthorized_User_is_forbidden (3.46s)
    --- PASS: TestSpireAuthorization/Valid_Certificate_Not_Spiffe_SNI (9.20s)
    --- PASS: TestSpireAuthorization/Certificate_Signed_by_unknown_authority (8.63s)
=== RUN   TestE2EServerRequestMuxNoAuth
=== RUN   TestE2EServerRequestMuxNoAuth/512k10p
=== RUN   TestE2EServerRequestMuxNoAuth/1m5p
--- PASS: TestE2EServerRequestMuxNoAuth (17.40s)
    --- PASS: TestE2EServerRequestMuxNoAuth/512k10p (5.60s)
    --- PASS: TestE2EServerRequestMuxNoAuth/1m5p (10.88s)
=== RUN   TestGetClientMonitoring
--- PASS: TestGetClientMonitoring (1.04s)
=== RUN   TestServerRequestYamuxPingPongFile
=== RUN   TestServerRequestYamuxPingPongFile/1k100p
=== RUN   TestServerRequestYamuxPingPongFile/512k10p
=== RUN   TestServerRequestYamuxPingPongFile/1M2p
=== RUN   TestServerRequestYamuxPingPongFile/100M1p
--- PASS: TestServerRequestYamuxPingPongFile (9.85s)
    --- PASS: TestServerRequestYamuxPingPongFile/1k100p (0.14s)
    --- PASS: TestServerRequestYamuxPingPongFile/512k10p (0.48s)
    --- PASS: TestServerRequestYamuxPingPongFile/1M2p (0.23s)
    --- PASS: TestServerRequestYamuxPingPongFile/100M1p (8.95s)
=== RUN   TestServerRequestNoMuxPingPongFile
=== RUN   TestServerRequestNoMuxPingPongFile/1k100p
=== RUN   TestServerRequestNoMuxPingPongFile/512k10p
=== RUN   TestServerRequestNoMuxPingPongFile/1M2p
=== RUN   TestServerRequestNoMuxPingPongFile/100M1p
--- PASS: TestServerRequestNoMuxPingPongFile (7.38s)
    --- PASS: TestServerRequestNoMuxPingPongFile/1k100p (0.08s)
    --- PASS: TestServerRequestNoMuxPingPongFile/512k10p (0.31s)
    --- PASS: TestServerRequestNoMuxPingPongFile/1M2p (0.16s)
    --- PASS: TestServerRequestNoMuxPingPongFile/100M1p (6.80s)
PASS
coverage: 62.8% of statements in ./../...
=== RUN   TestConcurrentStreams
--- PASS: TestConcurrentStreams (0.03s)
PASS
coverage: 11.4% of statements in ./../...
[1678863909] Helm Suite [helm] - 6/6 specs - 15 procs 
------------------------------
• [0.096 seconds]
On helm rendering template with default values set fails to render because endpoint is not set [unitTest]
/datos/work/edge-edgeproxy/helm/test/template_test.go:27

  Captured StdOut/StdErr Output >>
  << Captured StdOut/StdErr Output
------------------------------
• [0.100 seconds]
On helm rendering template with default values set with edgeproxy client authentication enabled when rendering configmap not have secret volume mounted if secretName is not set [unitTest]
/datos/work/edge-edgeproxy/helm/test/template_test.go:115

  Captured StdOut/StdErr Output >>
  << Captured StdOut/StdErr Output
------------------------------
• [0.105 seconds]
On helm rendering template with default values set when edgeproxy endpoint is set renders sucesfully [unitTest]
/datos/work/edge-edgeproxy/helm/test/template_test.go:41

  Captured StdOut/StdErr Output >>
   << Captured StdOut/StdErr Output
------------------------------
• [0.104 seconds]
On helm rendering template with default values set with edgeproxy client authentication enabled when rendering configmap have secret volume mounted if secretName is set [unitTest]
/datos/work/edge-edgeproxy/helm/test/template_test.go:111

  Captured StdOut/StdErr Output >>
  << Captured StdOut/StdErr Output
------------------------------
• [0.121 seconds]
On helm rendering template with default values set with edgeproxy client authentication enabled when rendering deployment resource have secret volume mounted if secretName is set [unitTest]
/datos/work/edge-edgeproxy/helm/test/template_test.go:81

  Captured StdOut/StdErr Output >>
  << Captured StdOut/StdErr Output
------------------------------
• [0.125 seconds]
On helm rendering template with default values set with edgeproxy client authentication enabled when rendering deployment resource not have secret volume mounted if secretName is not set [unitTest]
/datos/work/edge-edgeproxy/helm/test/template_test.go:95

  Captured StdOut/StdErr Output >>
  << Captured StdOut/StdErr Output
------------------------------
 SUCCESS! 184.922883ms 
coverage: [no statements]
=== RUN   TestYamuxHandleConnection
=== RUN   TestYamuxHandleConnection/Successful_proxied_connection
=== RUN   TestYamuxHandleConnection/Connection_closed_without_data_sent
--- PASS: TestYamuxHandleConnection (0.10s)
    --- PASS: TestYamuxHandleConnection/Successful_proxied_connection (0.00s)
    --- PASS: TestYamuxHandleConnection/Connection_closed_without_data_sent (0.10s)
=== RUN   TestEncodingFrames
--- PASS: TestEncodingFrames (0.00s)
=== RUN   TestInvalidRouterAction
--- PASS: TestInvalidRouterAction (0.00s)
=== RUN   TestReadFrame
--- PASS: TestReadFrame (0.00s)
PASS
coverage: 12.3% of statements in ./../...
=== RUN   TestSocksProxy
--- PASS: TestSocksProxy (0.03s)
=== RUN   TestToxicDialerDialLatency
--- PASS: TestToxicDialerDialLatency (0.81s)
=== RUN   TestTransparentProxy
--- PASS: TestTransparentProxy (0.00s)
PASS
coverage: 10.0% of statements in ./../../...
=== RUN   TestChainedAuthenticateOrder
=== RUN   TestChainedAuthenticateOrder/First_authenticate_engine_is_valid
=== RUN   TestChainedAuthenticateOrder/Second_authenticate_engine_is_the_only_one_valid
=== RUN   TestChainedAuthenticateOrder/all_authenticate_engine_are_not_valid
=== RUN   TestChainedAuthenticateOrder/all_authenticate_engine_but_secondary_is_executed_first
=== RUN   TestChainedAuthenticateOrder/secondary_fails_to_execute_but_first_is_valid
--- PASS: TestChainedAuthenticateOrder (0.00s)
    --- PASS: TestChainedAuthenticateOrder/First_authenticate_engine_is_valid (0.00s)
    --- PASS: TestChainedAuthenticateOrder/Second_authenticate_engine_is_the_only_one_valid (0.00s)
    --- PASS: TestChainedAuthenticateOrder/all_authenticate_engine_are_not_valid (0.00s)
    --- PASS: TestChainedAuthenticateOrder/all_authenticate_engine_but_secondary_is_executed_first (0.00s)
    --- PASS: TestChainedAuthenticateOrder/secondary_fails_to_execute_but_first_is_valid (0.00s)
=== RUN   TestPolicyEnforcerAuthorizeForwardGoodUser
=== RUN   TestPolicyEnforcerAuthorizeForwardGoodUser/allowed_ip:port
=== RUN   TestPolicyEnforcerAuthorizeForwardGoodUser/allowed_domain:port
=== RUN   TestPolicyEnforcerAuthorizeForwardGoodUser/not_allowed_ip:port
=== RUN   TestPolicyEnforcerAuthorizeForwardGoodUser/allowed_domain_not_allowed_ip:port
--- PASS: TestPolicyEnforcerAuthorizeForwardGoodUser (0.03s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardGoodUser/allowed_ip:port (0.00s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardGoodUser/allowed_domain:port (0.03s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardGoodUser/not_allowed_ip:port (0.00s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardGoodUser/allowed_domain_not_allowed_ip:port (0.00s)
=== RUN   TestPolicyEnforcerAuthorizeForwardBadUser
=== RUN   TestPolicyEnforcerAuthorizeForwardBadUser/not_allowed_ip:https_port
=== RUN   TestPolicyEnforcerAuthorizeForwardBadUser/not_allowed_ip:random_port
=== RUN   TestPolicyEnforcerAuthorizeForwardBadUser/not_allowed_domain:port
--- PASS: TestPolicyEnforcerAuthorizeForwardBadUser (0.01s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardBadUser/not_allowed_ip:https_port (0.00s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardBadUser/not_allowed_ip:random_port (0.00s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardBadUser/not_allowed_domain:port (0.00s)
=== RUN   TestPolicyEnforcerAuthorizeForwardUnknownUser
=== RUN   TestPolicyEnforcerAuthorizeForwardUnknownUser/not_allowed_ip:https_port
=== RUN   TestPolicyEnforcerAuthorizeForwardUnknownUser/not_allowed_ip:random_port
=== RUN   TestPolicyEnforcerAuthorizeForwardUnknownUser/not_allowed_domain:port
--- PASS: TestPolicyEnforcerAuthorizeForwardUnknownUser (0.00s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardUnknownUser/not_allowed_ip:https_port (0.00s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardUnknownUser/not_allowed_ip:random_port (0.00s)
    --- PASS: TestPolicyEnforcerAuthorizeForwardUnknownUser/not_allowed_domain:port (0.00s)
=== RUN   TestPolicyEnforcerConfigReload
=== RUN   TestPolicyEnforcerConfigReload/allowed_domain_not_allowed_ip:port
=== RUN   TestPolicyEnforcerConfigReload/allowed_domain_allowed_ip:port_after_config_reload
--- PASS: TestPolicyEnforcerConfigReload (1.01s)
    --- PASS: TestPolicyEnforcerConfigReload/allowed_domain_not_allowed_ip:port (0.00s)
    --- PASS: TestPolicyEnforcerConfigReload/allowed_domain_allowed_ip:port_after_config_reload (1.00s)
PASS
coverage: 18.1% of statements in ./../../...
composite coverage: 75.2% of statements

Ginkgo ran 9 suites in 1m22.810867035s
Test Suite Passed


NOTE: I unknown if is important but not all tests are yet migrated to ginkgo I still have standard go tests pending to migrate to ginkgo, Nevertheless I saw some of this not migrated are test generating some coverage.

Executing recursive but coverage for 1 package only

go test -count=1 -coverpkg=edgeproxy/cli -short -timeout 600s -coverprofile=build/gotest.out ./...

ginkgo -r -p --randomize-all --randomize-suites  --race --succinct --keep-going --timeout 600s --output-dir build/ -coverpkg=edgeproxy/cli -coverprofile=ginkgo.out ./...

758 vs 169 covered lines.

Let me now how can I help you to fix this, This is really blocking us the full migration to ginkgo at the moment :(

@onsi
Copy link
Owner

onsi commented Mar 15, 2023

ok thanks - let's focus on just the single coverpkg:

can you rerun this case:


go test -count=1 -coverpkg=edgeproxy/cli -short -timeout 600s -coverprofile=build/gotest.out ./...

ginkgo -r -p --randomize-all --randomize-suites  --race --succinct --keep-going --timeout 600s --output-dir build/ -coverpkg=edgeproxy/cli -coverprofile=ginkgo.out ./...

but with -coverpkg=./edgeproxy/cli instead of -coverpkg=edgeproxy/cli

@onsi
Copy link
Owner

onsi commented Mar 15, 2023

also - i appreciate some suites are ginkgo and others are testing.T tests. Do you have any individual suites where you mix ginkgo and testing.T? those will not work correctly when running with -p (that may or may not affect the coverage results)

also - is any of this open source so i can take a look or even try to run it locally?

@onsi
Copy link
Owner

onsi commented Mar 15, 2023

ah... wait a minute. I just tried this in a dummy repo myself and was confused about what you meant by "lines of code coverage" and now I see you mean "number of lines in the coverprofile" - which is an implementation detail and not giving you actual coverage. For that you need to use go tool cover with either -func (to generate a list) or -html (to generate an interactive coverage webpage). I find I get identical output when go tool cover is used - even though the generated coverprofiles are different.

in the course of digging into this, however, I did come across some inconsistent behavior in go where the coverage percentages are computed differently if one uses ./... vs explicitly naming a list of packages to test. I'm checking again with the latest version of Go and may need to open an issue.

@onsi
Copy link
Owner

onsi commented Mar 15, 2023

looks like go does in fact have some subtle issues when using coverpkg in this way:

golang/go#58770

i'm pretty sure ginkgo is doing ok at this point and that this is actually working now

@segator
Copy link
Author

segator commented Mar 15, 2023

maybe I wrongly understood the coverage.out file :) anyway there are something wrong, when I load this file to sonarQ it shows like less lines of code are covered when loading ginkgo coverage.out file compared to go test.

you did lot of questions hope I do not miss any :)

  • No I don't mix testing.T and ginkgo test, at the moment I only have 1 package covered with ginko rest of them are normal tests, but eventually will migrate all of them to ginkgo.
  • -coverpkg=./edgeproxy/cli instead of -coverpkg=edgeproxy/cli coverage.out is empty then seems this ./ is not working.
  • regarding opensource, unfortunatelly is my employer code so I can not share :/ but we can jump on a call and see the error in an screensharing if needed
  • not sure what you mean about go tool cover, teorically I can load the coverage.out directly to sonar it isn't?

thanks for all your work!

@onsi
Copy link
Owner

onsi commented Mar 15, 2023

yeah sorry for all the questions and thanks for your answers.

-coverpkg=./edgeproxy/cli instead of -coverpkg=edgeproxy/cli coverage.out is empty then seems this ./ is not working.

that is confusing to me - but i think the most efficient next step is to find some time to jump on a call and figure this out with you; which i'd be happy to do :)

i'm in Denver (so US mountain time zone) and my e-mail is onsijoe@gmail.com - send me an e-mail with some time options and we can figure something out.

Also (of course 😉) I do have one very last thing for you to try. instead of using file paths to specify coverpkg can you use package names? The packagename is how you import the package. For example to run coverage for all of Ginkgo I could:

ginkgo -coverpkg=github.com/onsi/ginkgo/v2/... -r

and you could:

ginkgo -coverpkg=github.com/your/repo/... -r

instead of ./...

And to do just a single package you could:

ginkgo -coverpkg=github.com/your/repo/edgeproxy/cli -r

The reason for all of this is because Ginkgo compiles and runs each test separately by first setting the working directory to the test and then running the test locally. this is why ./... doesn't work as it no longer points to the location you meant when you ran ginkgo. the fix i made was to detect ./ and rewrite the path so that it points back to where you want (this is why you see ./../... in some of your output. but it seems something about this isn't working for you and i'm not sure what. we can dig into it on the call.

but specifying a package path should always work and should always produce the coverage in sonarQ.

@segator
Copy link
Author

segator commented Mar 16, 2023

Hey Sorry for all confusions, 2.9.1 is working fine, I were confused as the content of coverage.out file is different generated by go test vs ginkgo, is just that go test have duplicated lines and I was asumming more lines in that file means more coverage, but it isn't.. as soon I loaded this file to sonar Coverage is just identical, Sorry to waste your time!
And thanks very much for your support and the quick fix!

@segator segator closed this as completed Mar 16, 2023
@onsi
Copy link
Owner

onsi commented Mar 16, 2023

hey no worries - glad you could figure it out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants