summaryrefslogtreecommitdiff
path: root/doc/development
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2018-06-21 14:57:58 +0000
committerRobert Speicher <robert@gitlab.com>2018-06-21 14:57:58 +0000
commite446e8712cd690981b48bc54ae1c20b7f4d3f99e (patch)
tree66f7e0944503101aac2212af7b16e29f14a36b39 /doc/development
parent8f99d30364e6f05662ce9430e0d46bc74e42aa2b (diff)
parentfda4d5540e4ce2631e317c1e256c221202900555 (diff)
downloadgitlab-ce-e446e8712cd690981b48bc54ae1c20b7f4d3f99e.tar.gz
Merge branch 'ce-6436-6149-unify-project_creator_spec' into 'master'
CE: Resolve "Extract EE specific files/lines for project_creator_spec" See merge request gitlab-org/gitlab-ce!19850
Diffstat (limited to 'doc/development')
-rw-r--r--doc/development/gotchas.md48
1 files changed, 48 insertions, 0 deletions
diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md
index 5786287d00c..d25d856c3a3 100644
--- a/doc/development/gotchas.md
+++ b/doc/development/gotchas.md
@@ -92,6 +92,54 @@ describe API::Labels do
end
```
+## Avoid using `allow_any_instance_of` in RSpec
+
+### Why
+
+* Because it is not isolated therefore it might be broken at times.
+* Because it doesn't work whenever the method we want to stub was defined
+ in a prepended module, which is very likely the case in EE. We could see
+ error like this:
+
+ 1.1) Failure/Error: allow_any_instance_of(ApplicationSetting).to receive_messages(messages)
+ Using `any_instance` to stub a method (elasticsearch_indexing) that has been defined on a prepended module (EE::ApplicationSetting) is not supported.
+
+### Alternative: `expect_next_instance_of`
+
+Instead of writing:
+
+```ruby
+# Don't do this:
+allow_any_instance_of(Project).to receive(:add_import_job)
+```
+
+We could write:
+
+```ruby
+# Do this:
+expect_next_instance_of(Project) do |project|
+ expect(project).to receive(:add_import_job)
+end
+```
+
+If we also want to expect the instance was initialized with some particular
+arguments, we could also pass it to `expect_next_instance_of` like:
+
+```ruby
+# Do this:
+expect_next_instance_of(MergeRequests::RefreshService, project, user) do |refresh_service|
+ expect(refresh_service).to receive(:execute).with(oldrev, newrev, ref)
+end
+```
+
+This would expect the following:
+
+```ruby
+# Above expects:
+refresh_service = MergeRequests::RefreshService.new(project, user)
+refresh_service.execute(oldrev, newrev, ref)
+```
+
## Do not `rescue Exception`
See ["Why is it bad style to `rescue Exception => e` in Ruby?"][Exception].