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.

Sign up for GitHub

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

Minitest: avoid installing at_exit #236

Merged
merged 5 commits into from
Jan 9, 2024
Merged

Minitest: avoid installing at_exit #236

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5836b96
ci: test minitest
3v0k4 Jan 5, 2024
8968646
fix(minitest): avoid installing at_exit
3v0k4 Jan 5, 2024
adc5412
ci: separate minitest tests in matrix
3v0k4 Jan 9, 2024
ce6f424
ci: reduce resource consumption
3v0k4 Jan 9, 2024
efa17d0
ci: consider different ruby versions as different builds
3v0k4 Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ jobs:
RAILS_ENV: test
RACK_ENV: test
KNAPSACK_PRO_ENDPOINT: https://api-staging.knapsackpro.com
KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC: 3fa64859337f6e56409d49f865d13fd7
KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC: $KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC
KNAPSACK_PRO_TEST_SUITE_TOKEN_MINITEST: $KNAPSACK_PRO_TEST_SUITE_TOKEN_MINITEST
EXTRA_TEST_FILES_DELAY: 10
- image: cimg/postgres:14.7
environment:
Expand Down Expand Up @@ -118,6 +119,9 @@ jobs:
export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--regular--split"
export KNAPSACK_PRO_RSPEC_SPLIT_BY_TEST_EXAMPLES=true
bundle exec rake knapsack_pro:rspec
- run:
working_directory: ~/rails-app-with-knapsack_pro
command: bundle exec rake knapsack_pro:minitest[--verbose]

integration-queue:
parameters:
Expand All @@ -135,7 +139,8 @@ jobs:
RAILS_ENV: test
RACK_ENV: test
KNAPSACK_PRO_ENDPOINT: https://api-staging.knapsackpro.com
KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC: 3fa64859337f6e56409d49f865d13fd7
KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC: $KNAPSACK_PRO_TEST_SUITE_TOKEN_RSPEC
KNAPSACK_PRO_TEST_SUITE_TOKEN_MINITEST: $KNAPSACK_PRO_TEST_SUITE_TOKEN_MINITEST
EXTRA_TEST_FILES_DELAY: 10
- image: cimg/postgres:14.7
environment:
Expand Down Expand Up @@ -211,6 +216,20 @@ jobs:
export KNAPSACK_PRO_TEST_DIR=turnip
export KNAPSACK_PRO_TEST_FILE_PATTERN="turnip/**/*.feature"
bundle exec rake "knapsack_pro:queue:rspec[-r turnip/rspec]"
- run:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are adding Minitest steps to the jobs that are supposed to be testing the matrix of Ruby & RSpec versions.

I think we could make it cleaner. You could add new jobs to the workflow for testing the matrix of Ruby & Minitest versions. Thanks to that:

  • we have the option to test different Minitest versions in the future.
  • we would avoid testing the matrix of different Minitest & RSpec versions because it does not make sense. The testing frameworks are independent.

You could create a new story for that to not hold off on the release of this fix.

workflows:
  tests:
    jobs:
      - unit
      - integration-regular-rspec:
          name: integration-regular__ruby-<< matrix.ruby >>__rspec-<< matrix.rspec >>
          matrix:
            parameters:
              ruby: ["3.0", "3.1", "3.2"]
              rspec: ["3.10.2", "3.11.0", "3.12.2"]
      - integration-queue-rspec:
          name: integration-queue__ruby-<< matrix.ruby >>__rspec-<< matrix.rspec >>
          matrix:
            parameters:
              ruby: ["3.0", "3.1", "3.2"]
              rspec: ["3.10.2", "3.11.0", "3.12.2"]
      - integration-regular-minitest:
          name: integration-regular__ruby-<< matrix.ruby >>__minitest-<< matrix.minitest >>
          matrix:
            parameters:
              ruby: ["3.0", "3.1", "3.2"]
              minitest: ["x.x.x", ...]
      - integration-queue-minitest:
          name: integration-queue__ruby-<< matrix.ruby >>__minitest-<< matrix.minitest >>
          matrix:
            parameters:
              ruby: ["3.0", "3.1", "3.2"]
              minitest: ["x.x.x", ...]

Copy link
Contributor Author

@3v0k4 3v0k4 Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ (I had to reduce the parallelism to avoid jobs being queued)

working_directory: ~/rails-app-with-knapsack_pro
command: |
# minitest ||
export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--queue--minitest"
export KNAPSACK_PRO_FIXED_QUEUE_SPLIT=true
bundle exec rake knapsack_pro:queue:minitest[--verbose]
- run:
working_directory: ~/rails-app-with-knapsack_pro
command: |
# minitest retry ||
export KNAPSACK_PRO_BRANCH="$CIRCLE_BRANCH--$CIRCLE_BUILD_NUM--queue--minitest"
export KNAPSACK_PRO_FIXED_QUEUE_SPLIT=true
bundle exec rake knapsack_pro:queue:minitest[--verbose]

workflows:
tests:
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### Unreleased (patch)

* fix(minitest): avoid installing `at_exit` (that would result in an empty run of Minitest after Knapsack Pro is finished in Queue Mode)

### 6.0.3

* fix(Turnip): make sure `.feature` files are recorded
Expand Down
4 changes: 4 additions & 0 deletions lib/knapsack_pro/runners/queue/minitest_runner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ class MinitestRunner < BaseRunner
def self.run(args)
require 'minitest'

# Avoid installing `at_exit` since we are calling `Minitest.run` ourselves.
# Without this, Minitest would run again (autorun) after `Minitest.run`.
::Minitest.class_variable_set('@@installed_at_exit', true)

ENV['KNAPSACK_PRO_TEST_SUITE_TOKEN'] = KnapsackPro::Config::Env.test_suite_token_minitest
ENV['KNAPSACK_PRO_QUEUE_RECORDING_ENABLED'] = 'true'
ENV['KNAPSACK_PRO_QUEUE_ID'] = KnapsackPro::Config::EnvGenerator.set_queue_id
Expand Down