Make integration_spec randomizable #296
Conversation
ellnix
force-pushed
the
make-integration-spec-randomizable
branch
from
72ee63f
to
9ada396
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #296 +/- ##
==========================================
+ Coverage 88.73% 89.04% +0.30%
==========================================
Files 10 10
Lines 657 657
==========================================
+ Hits 583 585 +2
+ Misses 74 72 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I will approve the changes @ellnix thanks for taking care of them!
I sent some comments but i think you can address them later!
spec/integration_spec.rb
Outdated
| @@ -365,6 +372,7 @@ | |||
| @products_in_database = Product.all | |||
|
|
|||
| Product.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) | |||
| sleep 5 | |||
There was a problem hiding this comment.
Oh no, the rebase thought this PR wanted to add the sleep back, I probably have the same problem in all the other PRs I rebased...
| Product.index.add_documents!(@palmpre.attributes.merge(id: -1)) | ||
| expect { Product.search('pal').to_json }.not_to raise_error | ||
| Product.index.delete_document!(-1) |
There was a problem hiding this comment.
I would recommend to use the xUnit structure whenever possible
| @@ -668,8 +687,7 @@ | |||
| describe 'Kaminari' do | |||
| before(:all) do | |||
| require 'kaminari' | |||
| MeiliSearch::Rails.configuration = { meilisearch_url: ENV.fetch('MEILISEARCH_HOST', 'http://127.0.0.1:7700'), | |||
There was a problem hiding this comment.
I guess that was not needed, good catch!
spec/integration_spec.rb
Outdated
| Movie.clear_index!(true) | ||
|
|
||
| 10.times { Movie.create(title: Faker::Movie.title) } | ||
|
|
||
| Movie.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) | ||
| sleep 5 |
There was a problem hiding this comment.
Since this PR #292 was merged, I guess you'll try to remove it later right?!
brunoocasali
added
the
maintenance
ellnix
force-pushed
the
make-integration-spec-randomizable
branch
from
4eac729
to
20fe6d8
Compare
ellnix
force-pushed
the
make-integration-spec-randomizable
branch
from
20fe6d8
to
8b3afd3
Compare
|
@brunoocasali Rebased, and I think I addressed all of the feedback. |
|
bors merge |
296: Make integration_spec randomizable r=ellnix a=ellnix # Pull Request ## Related issue Fixes #115 ## PR checklist Please check if your PR fulfills the following requirements: - [X] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)? - [X] Have you read the contributing guidelines? - [X] Have you made sure that the title is accurate and descriptive of the changes? # Notes (please read) - The behavior and methodology of the tests was left untouched wherever possible - Issue #115 instructs to create a PR for each fixed file but `spec/integration_spec.rb` was the only file with any issues, so this PR should fix the whole suite - Issue #115 does not fully instruct whether randomized testing should actually be enabled in the repo, so I left it disabled - I ran the suite ~40 times after fixing everything (probably another ~40 times while fixing it) so there could still be issues I did not spot. My rationale was that the tests should ideally be refactored before (or while) going through them line by line and trying to spot any more errors. Co-authored-by: ellnix <[email protected]>
|
Build failed: |
|
bors merge |
Pull Request
Related issue
Fixes #115
PR checklist
Please check if your PR fulfills the following requirements:
Notes (please read)
spec/integration_spec.rbwas the only file with any issues, so this PR should fix the whole suite