Trailing slash should be removed from allocator.test_dir in Knapsack::Runners::SpinachRunner

[rymai]

We, at GitLab, have an issue where the --features_path option passed to Spinach is set to features/: https://github.com/ArturT/knapsack/blob/e2e71d3974653bd3333fbf707fa20a8f0d95e209/lib/knapsack/runners/spinach_runner.rb#L15

The problem is that Spinach is not expecting a trailing slash for this option: https://github.com/codegram/spinach/blob/f561ec621dc309c87a60dd2c6ccca48a818ab298/lib/spinach/config.rb#L88

This ends in Spinach failing to require features/support/env.rb first: https://github.com/codegram/spinach/blob/f561ec621dc309c87a60dd2c6ccca48a818ab298/lib/spinach/runner.rb#L136-L139

I believe we should just call .chomp('/') on allocator.test_dir in Knapsack::Runners::SpinachRunner and move on with it. :)

For more detail about the issue we had, see https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10395#note_26752554.

[ArturT]

I will check that. Did you notice this after update of spinach gem?

For now you can manually set test_dir with KNAPSACK_TEST_DIR=features env var.

[rymai]

I will check that.

Thanks! ❤️

Did you notice this after update of spinach gem?

No, we're on Spinach 0.8.10 since more than 1.5 years! :)

For now you can manually set test_dir with KNAPSACK_TEST_DIR=features env var.

Yep, I figured this out, thanks!

[rymai]

Maybe a better fix would be @report_distributor.test_file_pattern.split('/').first as we do in Knapsack:: AllocatorBuilder#test_dir? I don't know the gem enough to know what's best, but I think both fix would solve the issue we have.

[ArturT]

I've released knapsack 1.13.3. Thanks for help!

BTW maybe you will find useful Queue Mode in knapsack_pro for auto-balancing CI nodes.
I wrote the article recently: http://docs.knapsackpro.com/2017/auto-balancing-7-hours-tests-between-100-parallel-jobs-on-ci-buildkite-example

[rymai]

Thanks @ArturT!