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.

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

Break up large spec files #12502

Merged
merged 4 commits into from
May 22, 2024
Merged

Break up large spec files #12502

merged 4 commits into from
May 22, 2024

Conversation

dacook
Copy link
Member

@dacook dacook commented May 21, 2024

What? Why?

Knapsack evenly distributes spec files to run in parallel, but can't break up spec files to run in parallel.
I noticed that a couple of large spec files were much longer than others, and therefore causing a bottleneck.

Also, I think we can change one of the runners to run more system specs, because the models are nice and quick.

What should we test?

  • green specs
  • quicker runtime
  • review the 'Usage' list to see individual runtimes (scroll to bottom left of Actions summary).

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

The orders file is too big and causes a bottleneck for parallelising specs.

Maybe they should be merged with the above specs, but I'm not familiar enough to know for sure.
@dacook dacook self-assigned this May 21, 2024
@dacook dacook added the technical changes only These pull requests do not contain user facing changes and are grouped in release notes label May 21, 2024
This was another large file, potentially causing a bottleneck.
All the order setup is duplicated from the other file which is a bit of shame, but I think it makes sense.
Each time we run a rails command, it can take some time to load up (I think it was 20s). We run two commands (db setup, then rspec), so the second one should be faster now.
@dacook
Copy link
Member Author

dacook commented May 21, 2024

One system_admin spec was still 7m, but maybe after this is merged, Knapsack will have better data to more evenly distribute specs.


Excerpt from logs shows that spring is used for both rake and rspec commands. I couldn't see timestamps to prove the time saving.

Run bin/rake db:create db:schema:load

Heads up! 🔥

👉 StimulusReflex requires caching to be enabled. Caching allows the session to be modified during ActionCable requests.

To enable caching in development, run:

  rails dev:cache

Running via Spring preloader in process 2883
Database 'open_food_network_test' already exists

Run bin/rake knapsack_pro:queue:rspec
Running via Spring preloader in process 2891

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Thank you! This is great.

@mkllnk mkllnk merged commit 5fdea3e into openfoodfoundation:master May 22, 2024
50 checks passed
@dacook
Copy link
Member Author

dacook commented May 22, 2024

Thanks, I just had a look at the latest master build, and it was done in 6m 19s.
The longest runner was 6m 9s, executing only one file spec/system/admin/order_spec.rb.
Second longest was 5m 24s, for spec/system/admin/subscriptions/crud_spec.rb.

So those are the next bottlenecks to attack..

@dacook
Copy link
Member Author

dacook commented May 22, 2024

FYI @filipefurtad0 I made a few tweaks to CI here ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical changes only These pull requests do not contain user facing changes and are grouped in release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants