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

Multiple failing tests #145

Closed
aparod opened this issue Nov 19, 2021 · 9 comments
Closed

Multiple failing tests #145

aparod opened this issue Nov 19, 2021 · 9 comments

Comments

@aparod
Copy link
Contributor

aparod commented Nov 19, 2021

The following tests are trying to match an :nxdomain error response, but when I run the tests I either get an :econnrefused error or a timeout from the ExUnit test itself after 60s:

test/faktory_worker/connection/connection_integration_test.exs:55
test/faktory_worker/connection/socket/tcp_test.exs:17
test/faktory_worker/connection/socket/ssl_test.exs:23

The following two tests fail because they require Faktory Enterprise features not present in the images being used in docker-compose.yml:

test/faktory_worker/job/batch_integration_test.exs:64
test/faktory_worker/job/batch_integration_test.exs:26

FWIW, I'm on a Macbook Pro, using Docker Compose v2.1.1, and I've made no changes to the code other than to add the services element to docker-compose.yml.

@Ch4s3
Copy link
Collaborator

Ch4s3 commented Jan 11, 2022

Is this resolved by #146 ?

@jeremyowensboggs
Copy link
Collaborator

I'm seeing test failures:

  1) test new!/1 creates a new batch (FaktoryWorker.BatchIntegrationTest)
     test/faktory_worker/job/batch_integration_test.exs:26
     ** (MatchError) no match of right hand side value: {:error, "The Batch subsystem is only available in Faktory Enterprise"}
     code: {:ok, bid} = Batch.new!(opts)
     stacktrace:
       test/faktory_worker/job/batch_integration_test.exs:33: (test)

.

  2) test open/2 allows opening a batch and adding new jobs (FaktoryWorker.BatchIntegrationTest)
     test/faktory_worker/job/batch_integration_test.exs:64
     ** (MatchError) no match of right hand side value: {:error, "The Batch subsystem is only available in Faktory Enterprise"}
     code: {:ok, bid} = Batch.new!(opts)
     stacktrace:
       test/faktory_worker/job/batch_integration_test.exs:71: (test)

@nickolaich
Copy link
Collaborator

nickolaich commented Jan 20, 2022

It wasn't resolved by #146
"The Batch subsystem is only available in Faktory Enterprise" is a response from Faktory and we can't automatically decide is there license or there is no.
couple quick ideas:

  • check some env/config variable and if there is no enterprise feature -> skip those tests
  • tag test with @enterprise and run tests except that tag if there is no license version of Faktory.
  • add to tests case Batch.new!(..) do and catch `{:error, "The Batch subsystem is only available in Faktory Enterprise"} , then write an error to log that enterprise features couldn't be tested.. Though error message may be changed in future.

@Ch4s3 @jeremyowensboggs I've added it tag :enterprise as part of #149

@aparod
Copy link
Contributor Author

aparod commented Jan 21, 2022

The docs say the tests should be run against the environment that is set up via docker-compose.yml. The Docker image being used there is contribsys/faktory:1.4.0, which is not an enterprise version of Faktory.

Since an enterprise binary of Faktory is available for download with every release, my thought was that a new image using the enterprise binary should be created and published to Docker Hub for folks to run tests against. My understanding is that you don't need a license key as long as you aren't running in production.

Does this seem like a reasonable approach?

@Ch4s3
Copy link
Collaborator

Ch4s3 commented Jan 24, 2022

@aparod that seems reasonable, but I'd want to talk to @mperham, to see how he want's people to approach testing Enterprise support in 3rd party clients.

@mperham
Copy link

mperham commented Jan 24, 2022

There is a macOS binary available. I’d suggest you use that to test things manually. I would prefer not to publish an Enterprise Linux container which is publicly available. You can use the output from INFO to determine if the server is running Enterprise.

@Ch4s3
Copy link
Collaborator

Ch4s3 commented Jan 25, 2022

I think @mperham's suggestion is the way to go. We can cordon off the enterprise tests and only run them locally if you have that binary or you're running your privately licensed copy. How does that sound to you @aparod?

@aparod
Copy link
Contributor Author

aparod commented Jan 26, 2022

Looks like we could tag the enterprise-related test modules and filter them out by default:

  1. Tag the relevant test modules with @moduletag :enterprise
  2. Add ExUnit.configure(exclude: :enterprise) to test_helper.exs
  3. Then, to include enterprise tests when running the suite: mix test --include enterprise

I'll write up a PR if that sounds agreeable to everyone.

@nickolaich
Copy link
Collaborator

@aparod I did it already in the opened PR #149 (except adding ExUnit.configure(exclude: :enterprise), thanks for hint, I absolutely forgot we can add it there).
I've just pushed changes to test_helper.exs. PR looks almost ready and you don't need to do anything else.

@aparod aparod closed this as completed Apr 7, 2023
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

5 participants