Improve NextInstanceOf stub_new
What does this MR do and why?
In spec/support/helpers/next_instance_of.rb
, when use expect_next_instance_of(klass, *new_args)
, it currently adds the :new
arguments validation check only when new_args.any?
.
This is not safe if new_args
only contains false
/nil
values. For example: new_args = [nil]
, new_args = [false, nil]
. For such values, new_args.any?
returns false
. As a result, we are NOT validating the :new
arguments at all.
We can use new_args.present?
. [nil].present?
returns true
. This ensures the stub_new
method adds the :new
arguments validation as we expected.
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
We can take spec/workers/destroy_pages_deployments_worker_spec.rb
to verify this.
- Check out
master
branch - Apply this diff:
diff --git a/spec/workers/destroy_pages_deployments_worker_spec.rb b/spec/workers/destroy_pages_deployments_worker_spec.rb
index 2c20c9004efa..efd074eaafc6 100644
--- a/spec/workers/destroy_pages_deployments_worker_spec.rb
+++ b/spec/workers/destroy_pages_deployments_worker_spec.rb
@@ -17,7 +17,7 @@
end
it 'can be called without last_deployment_id' do
- expect_next_instance_of(::Pages::DestroyDeploymentsService, project, nil) do |service|
+ expect_next_instance_of(::Pages::DestroyDeploymentsService, false, false) do |service|
expect(service).to receive(:execute).and_call_original
end
- run
bin/rspec ./spec/workers/destroy_pages_deployments_worker_spec.rb
. The spec still passes. This is not expected. - Check out this MR's branch. Repeat above step 2 and step 3, the spec will fail as expected:
Failures:
1) DestroyPagesDeploymentsWorker can be called without last_deployment_id
Failure/Error: ::Pages::DestroyDeploymentsService.new(project, last_deployment_id).execute
#<Pages::DestroyDeploymentsService (class)> received :new with unexpected arguments
expected: (false, false)
got: (#<Project id:67 namespace3/project3>>, nil)
Diff:
@@ -1 +1 @@
-[false, false]
+[#<Project id:67 namespace3/project3>>, nil]
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.