Amber: Test suite takes too long to run

[marksiemers]

Description

When running the test suite locally, it takes 2-5 minutes to complete. This is mostly due to a shards build on this line

Steps to Reproduce

1. crystal spec
2. wait ...
3. wait ...

Expected behavior: [What you expect to happen]

The full test suite to run more quickly on local development machine (under a minute, hopefully under 10 seconds)

Actual behavior: [What actually happens]

The full test suite takes several minutes to run.

Reproduces how often: [What percentage of the time does it reproduce?]

Every time you run crystal spec

Versions

amber v0.3.0

Additional Information

In gitter we discussed taking the following approach:

1. Local development, by default should NOT run shards build
2. Everywhere else (mostly Travis) it should run shards build by default
3. It should be easy to opt in on local development to running the shards build test from the command line

Either using an environment variable or option flag.

[marksiemers]

@eliasjpr @elorest - Please weigh in

[elorest]

That test proves the generated code actually works which I feel is important. Possibly having a flag to turn it off and on is a good idea. Tests take 1:28 minutes to complete as they are on my development box. Usually for development though I just run specific tests while developing and then the complete suite before pushing.

[marksiemers]

I think it depends on your internet connection:

With build

Finished in 2:25 minutes
336 examples, 0 failures, 0 errors, 0 pending

real    2m38.068s
user    1m28.255s
sys 0m13.785s

Without build:

Finished in 574.5 milliseconds
336 examples, 0 failures, 0 errors, 0 pending

real    0m19.855s
user    0m36.337s
sys 0m3.854s

[marksiemers]

I got some info from the Crystal gitter about how to possibly do this:
{% if flag?(:slow_test) %} require "./slow/*" {% end %}
and it sounds like -D allows you to pass option flags.

As an extension to this issue, I think isolating a build test file that is separate and isolated is a good idea. We can have it be a large integration test for the CLI - generate a new app, generate at least one of everything possible to generate, then build.

That way, if anything introduces a compile time error, it should show up.

[eliasjpr]

I noticed that the shards build test is compiling not only the test_app but also amber. I spend sometime to try to figure out the reason why is building amber. But maybe someone have insight of this already.

[drujensen]

@eliasjpr If you are referring the build target, this is used by docker to build the correct version of amber instead of trying to use a pre-built one and getting version mismatches.

https://github.com/amberframework/amber/blob/master/src/amber/cli/templates/app/Dockerfile.ecr#L16

[marksiemers]

Check the shard.yml file in a generated app. It has:

targets:
  ...

  amber:
    main: lib/amber/src/amber/cli.cr

I'm assuming that triggers a build.

[drujensen]

@marksiemers correct

[eliasjpr]

why do we have that? That can be remove no? That does not make sense to have in a generate app since the Amber CLI should be installed by the time the app is generated

[eliasjpr]

Addressed in the following repo and PRs

• Part of this was fixed with slimming the https://github.com/amberframework/amber-spec
• Clean Up and Dry Tests https://github.com/amberframework/amber/pull/344 with the addition of shards build test_app
• Move build test to its on file https://github.com/amberframework/amber/pull/354

[eliasjpr]

Resolved with #381 #354 #344