-
Notifications
You must be signed in to change notification settings - Fork 107
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
Lower the batch sizes to also test multiple batches being processed #283
Lower the batch sizes to also test multiple batches being processed #283
Conversation
859d6a2
to
75a78fc
Compare
Lower the batch sizes to also test multiple batches being processed
75a78fc
to
371fe86
Compare
Some comments on commit Ladas@371fe86 spec/models/manageiq/providers/amazon/aws_refresher_spec_common.rb
|
1 similar comment
Some comments on commit Ladas@371fe86 spec/models/manageiq/providers/amazon/aws_refresher_spec_common.rb
|
Checked commit Ladas@371fe86 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
# TODO(lsmola) extract the batch sizes to the settings and stub the settings instead | ||
# Lower batch sizes to test multiple batches for each collection | ||
allow_any_instance_of(ManagerRefresh::InventoryCollection).to receive(:batch_size).and_return(4) | ||
allow_any_instance_of(ManagerRefresh::InventoryCollection).to receive(:batch_size_pure_sql).and_return(4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dislike allow_any_instance_of
this is a good reason why the batch size should be configurable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM but lets make sure to follow up and make the batch_size configurable
Depends on:
Lower the batch sizes to also test multiple batches being processed