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

Change always passing visible false tests. #7905

Merged
merged 1 commit into from Jan 15, 2015

Conversation

5 participants
@cirosantilli
Contributor

cirosantilli commented Sep 29, 2014

Many usages of visible: false are wrong since visible: false matches both visible and invisible elements: http://rubydoc.info/github/jnicklas/capybara/master/Capybara/Node/Finders#all-instance_method

The correct way to check that an element is present but not visible is:

expect(find(...)).not_to be_visible

The two "I can't preview without text" scenarios are not true anymore: now you can preview without text, so I removed then. They only passed because of the misuse. Screenshot:

screenshot from 2014-09-30 15 56 00 js not preview visible without text

'I should see add a diff comment button' is simply wrong: it should be visible: true instead of visible: false.

@TeatroIO

This comment has been minimized.

TeatroIO commented Sep 29, 2014

I've prepared a stage. Click to open.

@@ -73,13 +73,13 @@ module SharedDiffNote
step 'I should not see the diff comment preview button' do
within(diff_file_selector) do
page.should have_css(".js-note-preview-button", visible: false)
page.should_not have_css(".js-note-preview-button", visible: true)

This comment has been minimized.

@houndci-bot

houndci-bot Sep 29, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

end
end
step 'I should not see the diff comment text field' do
within(diff_file_selector) do
page.should have_css(".js-note-text", visible: false)
page.should_not have_css(".js-note-text", visible: true)

This comment has been minimized.

@houndci-bot

houndci-bot Sep 29, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@@ -114,7 +114,7 @@ module SharedDiffNote
end
step 'I should see add a diff comment button' do
page.should have_css(".js-add-diff-note-button", visible: false)
page.should have_css(".js-add-diff-note-button", visible: true)

This comment has been minimized.

@houndci-bot

houndci-bot Sep 29, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@@ -131,7 +131,7 @@ module SharedDiffNote
step 'I should see the diff comment preview' do
within("#{diff_file_selector} form") do
page.should have_css(".js-note-preview", visible: false)
page.should have_css(".js-note-preview", visible: true)

This comment has been minimized.

@houndci-bot

houndci-bot Sep 29, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@@ -51,19 +51,19 @@ module SharedNote
step 'I should not see the comment preview' do
within(".js-main-target-form") do
page.should have_css(".js-note-preview", visible: false)
page.should_not have_css(".js-note-preview", visible: true)

This comment has been minimized.

@houndci-bot

houndci-bot Sep 29, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

end
end
step 'I should not see the comment preview button' do
within(".js-main-target-form") do
page.should have_css(".js-note-preview-button", visible: false)
page.should_not have_css(".js-note-preview-button", visible: true)

This comment has been minimized.

@houndci-bot

houndci-bot Sep 29, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

end
end
step 'I should not see the comment text field' do
within(".js-main-target-form") do
page.should have_css(".js-note-text", visible: false)
page.should_not have_css(".js-note-text", visible: true)

This comment has been minimized.

@houndci-bot

houndci-bot Sep 29, 2014

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@cirosantilli cirosantilli force-pushed the cirosantilli:visible-false branch from 572645f to 12cdbef Sep 29, 2014

@cirosantilli cirosantilli changed the title from Change always passing visible true tests to false. to [WIP] Change always passing visible true tests to false. Sep 30, 2014

@cirosantilli cirosantilli force-pushed the cirosantilli:visible-false branch from 12cdbef to 0f4b67b Sep 30, 2014

@cirosantilli cirosantilli changed the title from [WIP] Change always passing visible true tests to false. to Change always passing visible true tests to false. Sep 30, 2014

@cirosantilli cirosantilli force-pushed the cirosantilli:visible-false branch 3 times, most recently from 9b4895e to 1a27ebd Sep 30, 2014

@cirosantilli cirosantilli changed the title from Change always passing visible true tests to false. to Change always passing visible false tests. Oct 16, 2014

@jvanbaarsen

This comment has been minimized.

Contributor

jvanbaarsen commented Dec 23, 2014

@cirosantilli Can you make this mergeable again?

@cirosantilli cirosantilli force-pushed the cirosantilli:visible-false branch from 1a27ebd to 3ccd312 Jan 1, 2015

@cirosantilli cirosantilli changed the title from Change always passing visible false tests. to [WIP] Change always passing visible false tests. Jan 1, 2015

@cirosantilli cirosantilli force-pushed the cirosantilli:visible-false branch from 3ccd312 to 67b06e7 Jan 1, 2015

@cirosantilli cirosantilli changed the title from [WIP] Change always passing visible false tests. to Change always passing visible false tests. Jan 1, 2015

@cirosantilli

This comment has been minimized.

Contributor

cirosantilli commented Jan 1, 2015

@jvanbaarsen updated.

@jvanbaarsen jvanbaarsen added this to the 7.7 milestone Jan 4, 2015

@dzaporozhets dzaporozhets modified the milestones: 7.7, 7.8 Jan 13, 2015

@dzaporozhets dzaporozhets modified the milestones: 7.8, 7.7 Jan 13, 2015

dzaporozhets added a commit that referenced this pull request Jan 15, 2015

Merge pull request #7905 from cirosantilli/visible-false
Change always passing visible false tests.

@dzaporozhets dzaporozhets merged commit dbd54fa into gitlabhq:master Jan 15, 2015

2 checks passed

default Hound has reviewed the changes.
semaphoreci The build passed on Semaphore.
Details

@cirosantilli cirosantilli deleted the cirosantilli:visible-false branch Jan 16, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment