Add knapsack gem #390

enzofab91 · 2023-07-20T18:16:38Z

Board:

Ticket

Description:

This PR adds Knapsack gem and basic configuration.

lauperalti

Nice! We should probably add some doc/readme later of how it works and how to use it.

lauperalti · 2023-07-28T14:19:40Z

knapsack_rspec_report.json

+  "spec/requests/api/v1/feature_flags_spec.rb": 0.04056499999933294,
+  "spec/policies/admin/admin_user_policy_spec.rb": 0.004445000000487198,
+  "spec/requests/api/v1/settings_spec.rb": 0.04358700000011595
+}


New line missing

lauperalti · 2023-07-28T14:27:01Z

.github/workflows/ci.yml

    env:
      RAILS_ENV: test
      CC_TEST_REPORTER_ID: ${{ secrets.CC_TEST_REPORTER_ID }}
+      CI_NODE_TOTAL: 1


Shouldn't this be like a variable? Because it probably won't be 1 forever.

sebastiancaraballo · 2023-08-04T15:13:20Z

What's the status of this PR? Please add PR description

lauperalti · 2023-08-08T16:00:12Z

What's the status of this PR? Please add PR description

Hi @sebastiancaraballo. This PR is open and needs reviews to be merged on a "feature" branch release/improve-ci to configure together with parallel tests and improve CI.

sebastiancaraballo

Will this also work locally? Maybe it's worth adding the command on the README

sebastiancaraballo · 2023-08-08T17:58:53Z

knapsack_rspec_report.json

@@ -0,0 +1,31 @@
+{
+  "spec/requests/api/v1/passwords/edit_spec.rb": 0.18011699999988195,


Should this file be ignored? Just asking

santib · 2023-08-08T18:00:47Z

.github/workflows/ci.yml

      RAILS_ENV: test
      CC_TEST_REPORTER_ID: ${{ secrets.CC_TEST_REPORTER_ID }}
+      CI_NODE_TOTAL: 1
+      CI_NODE_INDEX: 0


same for this I guess?

santib · 2023-08-08T18:03:03Z

Gemfile

  gem 'annotate', '~> 3.2', '>= 3.0.3'
  gem 'dotenv-rails', '~> 2.7.6'
  gem 'factory_bot_rails', '~> 5.1', '>= 5.1.1'
+  gem 'knapsack', '~> 4.0.0', '>= 4.0.0'


Suggested change

gem 'knapsack', '~> 4.0.0', '>= 4.0.0'

gem 'knapsack', '~> 4.0'

maybe that's better so we allow any 4.x version?

santib · 2023-08-08T18:05:45Z

There are some logs in the CI that aren't very nice

========= Knapsack Time Offset Warning ==========
Time offset: 30s
Max allowed node time execution: 35s
Exceeded time: -27s
        
Global time execution for this CI node is fine.
Happy testing!

Need explanation? See FAQ:
https://docs.knapsackpro.com/ruby/knapsack#faq
=================================================
Read up on the benefits of a dynamic test split with Knapsack Pro Queue Mode:
https://docs.knapsackpro.com/2020/how-to-speed-up-ruby-and-javascript-tests-with-ci-parallelisation

Sign up for Knapsack Pro here:
https://knapsackpro.com/
=================================================

what is that Knapsack Time Offset warning?
can we remove the advertisment to purchase knapsack pro?

santib · 2023-08-08T18:08:55Z

Might be good to add some docs under docs/ directory explaining what it needs to be done, e.g. re-generating the knapsack_rspec_report.json from time to time

santib · 2023-08-10T17:56:53Z

.github/workflows/ci.yml

+      CI_NODE_TOTAL: ${{ secrets.CI_NODE_TOTAL || 1 }}
+      CI_NODE_INDEX: ${{ secrets.CI_NODE_INDEX || 0 }}


I don't think this will be in secrets, it might be set with a matrix or something like that I guess

santib · 2023-08-10T17:57:23Z

spec/spec_helper.rb

 require 'pundit/rspec'

+Knapsack.tracker.config(enable_time_offset_warning: false)
+


Suggested change

santib · 2023-08-14T18:42:24Z

.github/workflows/ci.yml

+        ci_node_total: [4]
+        # set N-1 indexes for parallel jobs
+        # When you run 2 parallel jobs then first job will have index 0, the second job will have index 1 etc
+        ci_node_index: [0, 1, 2, 3]


Great that you made it work, but for projects to start we might be good with just

Suggested change

ci_node_total: [4]

# set N-1 indexes for parallel jobs

# When you run 2 parallel jobs then first job will have index 0, the second job will have index 1 etc

ci_node_index: [0, 1, 2, 3]

ci_node_total: [1]

# set N-1 indexes for parallel jobs

# When you run 2 parallel jobs then first job will have index 0, the second job will have index 1 etc

ci_node_index: [0]

Okay we'll keep in this way to continue with our work/ tests but before merging into master we'll change and also we'll add the documentation explaining how to use

enzofab91 added 7 commits July 20, 2023 15:16

Add knapsack gem

80a5891

Add total nodes to ci workflow

d4672b1

empty commit to run CI

c0f6052

empty commit to run CI

62f8ea9

empty commit to run CI

18f2c6c

empty commit to run CI

a40547c

Update CI setup

691f400

lauperalti approved these changes Jul 28, 2023

View reviewed changes

enzofab91 added 5 commits August 1, 2023 09:56

Move CI_NODE_TOTAL to secrets

2e624f0

Move CI_NODE_TOTAL to secrets

27a4dc0

Move CI_NODE_TOTAL to secrets

fc2d158

Move CI_NODE_TOTAL to secrets

0ebfcb9

Move CI_NODE_TOTAL to secrets

51eb9f8

Fix

b685aa8

enzofab91 changed the base branch from main to release/improve-ci August 8, 2023 15:30

enzofab91 requested a review from santib August 8, 2023 16:02

sebastiancaraballo approved these changes Aug 8, 2023

View reviewed changes

santib reviewed Aug 8, 2023

View reviewed changes

santib approved these changes Aug 8, 2023

View reviewed changes

Remove advertisements and warning when running CI

57e4b7a

rodrieiz force-pushed the add_knapsack_gem branch from d89ef1b to a03ac71 Compare August 10, 2023 15:16

move harcoded values to secrets with a default value

316918a

rodrieiz force-pushed the add_knapsack_gem branch from a03ac71 to 316918a Compare August 10, 2023 15:19

santib reviewed Aug 10, 2023

View reviewed changes

spec/spec_helper.rb Outdated

require 'pundit/rspec'

Knapsack.tracker.config(enable_time_offset_warning: false)

Copy link

Member

santib Aug 10, 2023

Choose a reason for hiding this comment

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

Suggested change

rodrieiz added 3 commits August 11, 2023 15:32

try to set matrix strategy

bda5aca

queue mode works?

fa62ac7

vars

8b6591e

rodrieiz force-pushed the add_knapsack_gem branch from e5d3d29 to 8b6591e Compare August 14, 2023 15:44

santib reviewed Aug 14, 2023

View reviewed changes

Merge branch 'release/improve-ci' into add_knapsack_gem

91d7b17

grosendo2006 mentioned this pull request Aug 15, 2023

Add parallel gem #393

Merged

Merge branch 'release/improve-ci' into add_knapsack_gem

c21414b

enzofab91 merged commit 171f2ba into release/improve-ci Aug 15, 2023

enzofab91 deleted the add_knapsack_gem branch August 15, 2023 15:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add knapsack gem #390

Add knapsack gem #390

Uh oh!

enzofab91 commented Jul 20, 2023 •

edited

Loading

Uh oh!

lauperalti left a comment

Uh oh!

lauperalti Jul 28, 2023

Uh oh!

lauperalti Jul 28, 2023

Uh oh!

sebastiancaraballo commented Aug 4, 2023

Uh oh!

lauperalti commented Aug 8, 2023 •

edited

Loading

Uh oh!

sebastiancaraballo left a comment

Uh oh!

sebastiancaraballo Aug 8, 2023

Uh oh!

santib Aug 8, 2023

Uh oh!

santib Aug 8, 2023

Uh oh!

santib commented Aug 8, 2023

Uh oh!

santib commented Aug 8, 2023

Uh oh!

santib Aug 10, 2023

Uh oh!

santib Aug 10, 2023

Uh oh!

santib Aug 14, 2023

Uh oh!

rodrieiz Aug 14, 2023

Uh oh!

Uh oh!

		@@ -0,0 +1,31 @@
		{
		"spec/requests/api/v1/passwords/edit_spec.rb": 0.18011699999988195,

	gem 'knapsack', '~> 4.0.0', '>= 4.0.0'
	gem 'knapsack', '~> 4.0'

		CI_NODE_TOTAL: ${{ secrets.CI_NODE_TOTAL \|\| 1 }}
		CI_NODE_INDEX: ${{ secrets.CI_NODE_INDEX \|\| 0 }}

		require 'pundit/rspec'

		Knapsack.tracker.config(enable_time_offset_warning: false)

Add knapsack gem #390

Add knapsack gem #390

Uh oh!

Conversation

enzofab91 commented Jul 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Board:

Description:

Uh oh!

lauperalti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebastiancaraballo commented Aug 4, 2023

Uh oh!

lauperalti commented Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sebastiancaraballo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

santib commented Aug 8, 2023

Uh oh!

santib commented Aug 8, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

enzofab91 commented Jul 20, 2023 •

edited

Loading

lauperalti commented Aug 8, 2023 •

edited

Loading