summaryrefslogtreecommitdiff
path: root/doc/development/gotchas.md
diff options
context:
space:
mode:
Diffstat (limited to 'doc/development/gotchas.md')
-rw-r--r--doc/development/gotchas.md127
1 files changed, 84 insertions, 43 deletions
diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md
index b25ce79e89f..0f78e8238af 100644
--- a/doc/development/gotchas.md
+++ b/doc/development/gotchas.md
@@ -32,71 +32,112 @@ spec/models/user_spec.rb|6 error| Failure/Error: u = described_class.new NoMeth
Except for the top-level `describe` block, always provide a String argument to
`describe`.
-## Don't `rescue Exception`
+## Don't assert against the absolute value of a sequence-generated attribute
-See ["Why is it bad style to `rescue Exception => e` in Ruby?"][Exception].
+Consider the following factory:
-_**Note:** This rule is [enforced automatically by
-Rubocop](https://gitlab.com/gitlab-org/gitlab-ce/blob/8-4-stable/.rubocop.yml#L911-914)._
+```ruby
+FactoryGirl.define do
+ factory :label do
+ sequence(:title) { |n| "label#{n}" }
+ end
+end
+```
-[Exception]: http://stackoverflow.com/q/10048173/223897
+Consider the following API spec:
-## Don't use inline JavaScript in views
+```ruby
+require 'rails_helper'
-Using the inline `:javascript` Haml filters comes with a
-performance overhead. Using inline JavaScript is not a good way to structure your code and should be avoided.
+describe API::Labels do
+ it 'creates a first label' do
+ create(:label)
-_**Note:** We've [removed these two filters](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/initializers/hamlit.rb)
-in an initializer._
-
-### Further reading
+ get api("/projects/#{project.id}/labels", user)
-- Stack Overflow: [Why you should not write inline JavaScript](http://programmers.stackexchange.com/questions/86589/why-should-i-avoid-inline-scripting)
+ expect(response).to have_http_status(200)
+ expect(json_response.first['name']).to eq('label1')
+ end
-## ID-based CSS selectors need to be a bit more specific
+ it 'creates a second label' do
+ create(:label)
-Normally, because HTML `id` attributes need to be unique to the page, it's
-perfectly fine to write some JavaScript like the following:
+ get api("/projects/#{project.id}/labels", user)
-```javascript
-$('#js-my-selector').hide();
+ expect(response).to have_http_status(200)
+ expect(json_response.first['name']).to eq('label1')
+ end
+end
```
-However, there's a feature of GitLab's Markdown processing that [automatically
-adds anchors to header elements][ToC Processing], with the `id` attribute being
-automatically generated based on the content of the header.
-
-Unfortunately, this feature makes it possible for user-generated content to
-create a header element with the same `id` attribute we're using in our
-selector, potentially breaking the JavaScript behavior. A user could break the
-above example with the following Markdown:
+When run, this spec doesn't do what we might expect:
-```markdown
-## JS My Selector
-```
+```sh
+1) API::API reproduce sequence issue creates a second label
+ Failure/Error: expect(json_response.first['name']).to eq('label1')
-Which gets converted to the following HTML:
+ expected: "label1"
+ got: "label2"
-```html
-<h2>
- <a id="js-my-selector" class="anchor" href="#js-my-selector" aria-hidden="true"></a>
- JS My Selector
-</h2>
+ (compared using ==)
```
-[ToC Processing]: https://gitlab.com/gitlab-org/gitlab-ce/blob/8-4-stable/lib/banzai/filter/table_of_contents_filter.rb#L31-37
+That's because FactoryGirl sequences are not reseted for each example.
+
+Please remember that sequence-generated values exist only to avoid having to
+explicitly set attributes that have a uniqueness constraint when using a factory.
### Solution
-The current recommended fix for this is to make our selectors slightly more
-specific:
+If you assert against a sequence-generated attribute's value, you should set it
+explicitly. Also, the value you set shouldn't match the sequence pattern.
+
+For instance, using our `:label` factory, writing `create(:label, title: 'foo')`
+is ok, but `create(:label, title: 'label1')` is not.
+
+Following is the fixed API spec:
-```javascript
-$('div#js-my-selector').hide();
+```ruby
+require 'rails_helper'
+
+describe API::Labels do
+ it 'creates a first label' do
+ create(:label, title: 'foo')
+
+ get api("/projects/#{project.id}/labels", user)
+
+ expect(response).to have_http_status(200)
+ expect(json_response.first['name']).to eq('foo')
+ end
+
+ it 'creates a second label' do
+ create(:label, title: 'bar')
+
+ get api("/projects/#{project.id}/labels", user)
+
+ expect(response).to have_http_status(200)
+ expect(json_response.first['name']).to eq('bar')
+ end
+end
```
+## Don't `rescue Exception`
+
+See ["Why is it bad style to `rescue Exception => e` in Ruby?"][Exception].
+
+_**Note:** This rule is [enforced automatically by
+Rubocop](https://gitlab.com/gitlab-org/gitlab-ce/blob/8-4-stable/.rubocop.yml#L911-914)._
+
+[Exception]: http://stackoverflow.com/q/10048173/223897
+
+## Don't use inline JavaScript in views
+
+Using the inline `:javascript` Haml filters comes with a
+performance overhead. Using inline JavaScript is not a good way to structure your code and should be avoided.
+
+_**Note:** We've [removed these two filters](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/initializers/hamlit.rb)
+in an initializer._
+
### Further reading
-- Issue: [Merge request ToC anchor conflicts with tabs](https://gitlab.com/gitlab-org/gitlab-ce/issues/3908)
-- Merge Request: [Make tab target selectors less naive](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2023)
-- Merge Request: [Make cross-project reference's clipboard target less naive](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2024)
+- Stack Overflow: [Why you should not write inline JavaScript](http://programmers.stackexchange.com/questions/86589/why-should-i-avoid-inline-scripting)