Skip to content

Speed up CI: faster test job startup and runtime-balanced spec groups - #1628

Merged
jrhoads merged 3 commits into
masterfrom
replace-localstack-with-floci
Oct 27, 2026
Merged

jrhoads merged 3 commits into
masterfrom
replace-localstack-with-floci

Conversation

@jrhoads

@jrhoads jrhoads commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Purpose

Make the CI test suite finish faster and use fewer runner minutes: start every test job faster, and spread the specs evenly across fewer jobs.

Approach

Every parallel test job spends about 1m20s setting up before any spec runs. Most of that is GitHub Actions pulling the service images one after another and waiting for them to report healthy. LocalStack, used only as an S3 stand-in, was the slowest service in each job: about 24s to pull its 1.78 GB image, then about 13s waiting for its health check. That cost was paid by all 16 jobs on every run.

This PR replaces LocalStack with Floci, a LocalStack-compatible S3 emulator on the same port 4566. Its image is about 346 MB, and it reports healthy in under a second. That cuts setup time in every test job. The app talks to it through the same AWS_ENDPOINT_URL_S3 settings, so no app code changes.

Specs were also split poorly: parallel_test grouped them by file count, so job times ranged from about 10s to 165s while every job paid the full setup cost. Specs are now grouped by recorded runtime across 8 jobs instead of 16. One spec (events_preloader_spec.rb) alone took about 80s, which set the minimum time for any job, so it has been rewritten to test the same behaviour in under a second.

Key Modifications

  • .github/workflows/parallel_ci.yml:
    • Replaced the localstack service with floci (floci/floci:2.3.0) to cut setup time in every test job.
    • Set METADATA_STORAGE_BUCKET_NAME: metadata in the Test step, so the S3 metadata storage specs run against the emulator again. They have been skipped in CI since that variable started defaulting to "".
    • Reduced the matrix from 16 to 8 jobs and switched parallel_test to --group-by runtime --runtime-log spec/parallel_runtime_rspec.log.
    • Each job now records per-file runtimes (ParallelTests::RSpec::RuntimeLogger) and uploads them as a rspec-runtime-<index> artifact (14-day retention) after a successful run.
  • spec/parallel_runtime_rspec.log (new): per-file runtimes used to balance the groups. It is seeded from a full local run.
  • spec/services/events_preloader_spec.rb: the chunking spec used to create 1,100 DOIs (~80s). It now uses stub_const to set CHUNK_SIZE to 2 and reuses the spec's three DOIs. It also asserts what the old spec did not: the preloader makes one query per chunk, and an event spanning two chunks is attached exactly once to each DOI.

Important Technical Details

  • Expected effect (estimated from local timings, to be confirmed by this PR's run):
    • With runtime grouping, each of the 8 jobs should have about 87s of spec time. That floor is now set by spec/requests/datacite_dois/get_spec.rb, the slowest remaining file.
    • Total runner time should roughly halve compared with 16 jobs.
  • Local verification:
    • The full suite (2,223 examples) passes locally with Floci and the metadata bucket enabled.
    • Removing all_events.uniq! from EventsPreloader makes the new chunking spec fail.
  • Refreshing the runtime log: after a green CI run, download the artifacts and merge them:
    gh run download <run-id> -p 'rspec-runtime-*' -D tmp/rspec-runtime
    cat tmp/rspec-runtime/*/parallel_runtime_rspec.log | sort > spec/parallel_runtime_rspec.log
    Spec files with no entry in the log are assigned the average runtime, so new specs don't break grouping.
  • Ordering: runtime grouping changes which spec files run together in a job. Any order-dependent flakiness that was previously hidden could surface.
  • Out of scope: local docker-compose*.yml still uses LocalStack. Switching those is left for a follow-up.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

🤖 Generated with Claude Code

jrhoads and others added 2 commits October 23, 2026 10:20
- Replace the LocalStack service with Floci (floci/floci:2.3.0), a much
  smaller LocalStack-compatible emulator that is ready almost instantly.
- Set METADATA_STORAGE_BUCKET_NAME in CI so the S3 metadata storage path
  is exercised again (it has been off since the env var defaulted to "").
- Split specs into 8 jobs grouped by recorded runtime
  (spec/parallel_runtime_rspec.log) instead of 16 jobs grouped by file
  count. Each job uploads its runtimes so the log can be refreshed.
- Rewrite the EventsPreloader chunking spec to stub CHUNK_SIZE instead of
  creating 1,100 DOIs (80s -> <1s), and assert that events spanning chunks
  are attached once to each DOI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tests no longer wait for lint to pass, so results arrive sooner and a lint
or bundler-audit failure doesn't hide test results. A new push to a PR now
cancels the run still going on the previous commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jrhoads
jrhoads requested a review from a team October 23, 2026 15:31
Per-file runtimes from the first green 8-job run on this branch
(actions run 39939499539), merged from the rspec-runtime-* artifacts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this log file be checked in?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. It is used in the specs to distribute the tests based on total runtime

@jrhoads
jrhoads requested a review from a team October 27, 2026 12:38

@richardhallett richardhallett left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess fine, the change to floci is interesting choice, not that I'm in love with localstack, but for CI I guess we can give it a go.

@jrhoads
jrhoads merged commit e6de800 into master Oct 27, 2026
10 checks passed
@jrhoads
jrhoads deleted the replace-localstack-with-floci branch October 27, 2026 12:41
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

Successfully merging this pull request may close these issues.

3 participants