faradayio / cage

Develop and deploy complex Docker applications
http://cage.faraday.io
Apache License 2.0
307 stars 26 forks source link

`cage test <service>` can be misused #65

Open erithmetic opened 7 years ago

erithmetic commented 7 years ago

We created cage test <service> for our own needs, but if we want cage to be an opinionated framework for multi-container apps that encourages best practices, we need to make this functionality more of a customization than an integral/recommended feature.

The recommended setup ought to be that each service can run its own unit and integration tests independently from the rest of the other services.

Of course, you'd still want a test target that can run acceptance tests and any focused integration tests between a few services. But this test mode should not be relied on for independent service unit tests.

Example: We have a rails app called web. Inside there is a docker-compose.yml:

test:
  command: rake
  build: .
  volumes:
    - ./:/app
  environment:
    RAILS_ENV: test
    DATABASE_URL: postgres://postgres@db/web_test
  depends_on:
    - db

db:
  image: postgresql

prepare:
  extends: test
  command: rake db:setup

And you can run tests with docker-compose run prepare and then docker-compose run test or individual RSpec tests with docker-compose run test rspec spec/foo_spec.rb --fail-fast

The docker-compose.yml can be created by a future plugin system for generating apps.

seamusabshere commented 7 years ago

@dkastner "antipattern" is the wrong word

we're paving a cowpath, which is something that cage does in a lot of ways

in fact, a lot of what cage does could be done by docker-compose, but they refuse to upstream it, so we take an explicitly more open philosophy

emk commented 7 years ago

I believe that:

  1. There should be per-service unit tests. There may also be many other kinds of tests, but per-service unit tests are essential.
  2. There should be a standard way to invoke per-service unit tests. This should not be provided by a plugin or extension system. Testing should always be first class.
  3. Other, more complicated kinds of tests can be run via cage run and task containers. If somebody has a kind of test that can't be run this way, I'd love to see the use case and study it.
  4. As other kinds of testing become well understood, I'm happy to add specific support for them in cage. More built-in testing hooks rather than less, as long as they make coherent sense and are based on formalizing real-world best practices.

The biggest limitation here is probably things like Rust containers, where there might be a two-container build process with no development tools in the final container. (Well, Alpine supports Rust now, so there are also one-container options.) But this raises all kinds of issues that I haven't yet figured out yet. I'm waiting for practical experience with an actual Rust cage service that I'm working on.

erithmetic commented 7 years ago

@seamusabshere

First, what we're doing here with a shared test configuration is not something docker-compose refuses to upstream. There are other features cage provides where that is the case.

Second, this is a dangerous cowpath that we should not encourage others to follow. Requiring other services to run in order to run your unit tests adds a lot of complexity.

@emk

There should be per-service unit tests

Obviously, I agree with you here. But not that they should be run via a cage command.

There should be a standard way to invoke per-service unit tests

I disagree but I can see an alternative where we keep cage test <service> but drop the expectation that that command will run inside a container running in the test target. Instead, you can configure the io.fdy.cage.test tag to simply exec the docker-compose run test command.

The only reason we need cage test <service> at this point is because some services share a database and schema. This practice is widely considered an antipattern. Without this use case, there really is no reason to have a test target. Within each app you can simply run tests locally outside of a container or configure a local, per-app docker-compose.yml dedicated to running tests. cage's job is to manage the development, staging, and production targets and provide an acceptance testing/integration testing platform.

erithmetic commented 7 years ago

In fact, I'd even go so far as to argue that we no longer need cage test <service>, even for our use case.

erithmetic commented 7 years ago

In the end, I'd rather just have a bare cage test command that runs acceptance tests and that's it.

seamusabshere commented 7 years ago

This practice is widely considered an antipattern.

other things widely considered antipatterns that are still good ideas for some organizations because of culture or history:

cage knowing about placeholders / shared schemas makes sense for Faraday. It may be a "smell" that causes you to investigate things, but whether it's an antipattern is moot.

emk commented 7 years ago

The only reason we need cage test <service> at this point is because some services share a database and schema. Without this use case, there really is no reason to have a test target.

From my perspective, that's definitely not why we have cage test <service>. The reason why we have cage test <service> is that I want some standard way to run any per-service unit tests without having to remember the differences between RSpec, Test::Unit, mocha, npm test, cargo test and every other flavor that can appear in a container.

This is also because I feel that if we have a cage run command or a cage up command, we should also have a corresponding cage test. Testing should always be first-class, equally important as running things.

I am perfectly OK with a bare cage test running acceptance tests! We have bare cage up; we should have bare cage test. This could either: (1) run a task pod named test, if any, or (2) issue a configurable run command, they way cage up --init does. I'm leaning towards (1).