components-contrib icon indicating copy to clipboard operation
components-contrib copied to clipboard

optimize: add e2etests buildtags for e2e

Open 1046102779 opened this issue 2 years ago • 8 comments

=== RUN   TestAuthentication
=== RUN   TestAuthentication/Valid_jwt_and_valid_seed
err: nats: no servers available for connection
    authentication_test.go:56: Did not expect error during connect nats: no servers available for connection
=== RUN   TestAuthentication/Valid_jwt_and_invalid_seed
err: dial tcp [::1]:4222: connect: network is unreachable
=== RUN   TestAuthentication/Invalid_jwt_and_valid_seed
err: nats: no servers available for connection
--- FAIL: TestAuthentication (0.00s)
    --- FAIL: TestAuthentication/Valid_jwt_and_valid_seed (0.00s)
    --- PASS: TestAuthentication/Valid_jwt_and_invalid_seed (0.00s)
    --- PASS: TestAuthentication/Invalid_jwt_and_valid_seed (0.00s)
FAIL
FAIL    github.com/dapr/components-contrib/tests/e2e/pubsub/jetstream/authentication    0.009s
FAIL

1046102779 avatar Aug 10 '22 02:08 1046102779

Codecov Report

Merging #1951 (0337854) into master (0ffa1f3) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1951   +/-   ##
=======================================
  Coverage   37.41%   37.41%           
=======================================
  Files         191      191           
  Lines       23824    23824           
=======================================
  Hits         8913     8913           
  Misses      14148    14148           
  Partials      763      763           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 10 '22 02:08 codecov[bot]

Normally, tests in the e2e folder don't run when running make test.

However, I understand the issue and I think it should be fixed. But, if the server isn't ready, the test should be skipped entirely and not just avoiding a fatal log.

See how binding.ipfs does it: if you want to run the test, you need to pass an environmental variable; otherwise, the entire test is skipped:

https://github.com/dapr/components-contrib/blob/master/bindings/ipfs/ipfs_test.go#L36-L43

ItalyPaleAle avatar Aug 10 '22 04:08 ItalyPaleAle

If this is an integration test I think we should use build tags! Please don't use environment variables to trigger the test unless this is needed for configuring the server etc, still I think build tags are the way to go.

FYI @ItalyPaleAle

berndverst avatar Aug 10 '22 05:08 berndverst

/cc @berndverst @ItalyPaleAle @daixiang0 . Thanks, i added a e2etest buildtag for the unit test jetstream file.

1046102779 avatar Aug 10 '22 07:08 1046102779

Please change to use an env var to make the tests run. See example in bindings.ipfs: https://github.com/dapr/components-contrib/blob/master/bindings/ipfs/ipfs_test.go#L36-L43

IPFS_TEST: "1"

      if utils.IsTruthy(os.Getenv("IPFS_TEST")) {
                  log.Println("IPFS_TEST env var is not set to a truthy value; skipping tests")
                  os.Exit(0)
      }

IPFS_TEST=1 don't execute this unit test. Why not 0?

1046102779 avatar Aug 10 '22 08:08 1046102779

IPFS_TEST=1 don't execute this unit test. Why not 0?

It should! Did you export IPFS_TEST=1?

If this is an integration test I think we should use build tags! Please don't use environment variables to trigger the test unless this is needed for configuring the server etc, still I think build tags are the way to go.

Whatever the boss (@berndverst) says :) However, the downside of build tags is that you need to configure your IDE too or the language server won't scan them...

ItalyPaleAle avatar Aug 10 '22 16:08 ItalyPaleAle

@ItalyPaleAle The env variable IPFS_TEST is a neutral word, if change to IPFS_TEST_OFF is more appropriate?

1046102779 avatar Aug 11 '22 03:08 1046102779

Oh I see now. It is missing a ! before utils.IsTruthy. Want to open another PR to fix that?

ItalyPaleAle avatar Aug 11 '22 03:08 ItalyPaleAle

Oh I see now. It is missing a ! before utils.IsTruthy. Want to open another PR to fix that?

hah, i do it.

1046102779 avatar Aug 11 '22 03:08 1046102779

Once we create certification tests for this component we should move this into a certification test instead! Does someone know how to enable NATS jetstream with a Docker image? Or ideally how to create the Docker-compose file? A lot of configuration seems to be required there.

berndverst avatar Aug 11 '22 23:08 berndverst

Once we create certification tests for this component we should move this into a certification test instead! Does someone know how to enable NATS jetstream with a Docker image? Or ideally how to create the Docker-compose file? A lot of configuration seems to be required there.

We may add readme and makefile in this e2e example.

1046102779 avatar Aug 12 '22 01:08 1046102779