Skip to content
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

AO3-4702 Test autocomplete #2601

Closed
wants to merge 7 commits into from
Closed

AO3-4702 Test autocomplete #2601

wants to merge 7 commits into from

Conversation

zz9pzza
Copy link
Contributor

@zz9pzza zz9pzza commented Oct 27, 2016

Pull Request Checklist

  • Have you read How to write the perfect pull request?
  • Have you read through the contributor guidelines?
  • Have you added tests for any changed functionality?
  • Have you added the JIRA issue number as the first thing in your pull request title (eg: AO3-1234 Fix thing)
  • Have you updated the JIRA issue with the information below?

Issue

https://otwarchive.atlassian.net/browse/AO3-4702

Purpose

Test the autocomplete.

Testing

No manual testing required.

find(selector).native.send_keys(contents)
#sleep 5
#puts page.evaluate_script( 'document.documentElement.innerHTML' )
puts find(:css , 'div.autocomplete.dropdown', :wait=> 20 , :text => 'uby' ).text
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what it's worth, the text that autocomplete generates is actually inside a li inside a ul inside the div:

<div class="autocomplete dropdown" style="position: absolute; z-index: 999; display: none;">
  <ul role="listbox" aria-activedescendant="ui-active-menuitem" style="display: block;">
    <li role="option" class="odd"><b>Te</b>sting</li>

So I don't think looking for it in the div will work -- I think you'll have to look at the list items themselves with something like div.autocomplete li

@@ -2,7 +2,7 @@

Feature: Tag Wrangling - Fandoms

Scenario: fandoms wrangling - syns, mergers, autocompletes, metatags
Scenario: fandoms wrangling - syns, mergerss, metatags
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a typo and should be "mergers", not "mergerss"

And I am logged in as "Enigel" with password "wrangulate!"
When I go to the "Ruby (SU)" tag page
And I follow "Edit"
And I type in "ul.autocomplete" with "Ruby"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ul.autocomplete" is the container, "ul.autocomplete input" is the form field to type into

@@ -298,6 +320,7 @@ def with_scope(locator)
sleep 120
end


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line detected.

# resist the temptation. You should only verify outcome that is observable for
# the user (or external system) and databases usually are not.
result = false
(1..20).each do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Integer#times for a simple loop which iterates a fixed number of times.

@@ -33,6 +33,11 @@ def with_scope(locator)
page.driver.browser.url_whitelist = ['http://127.0.0.1']
end

When /^I type in "([^"]*)" with "([^"]*)"$/ do |selector,contents|
Rails.cache.delete('/v1/autocomplete_hack/last_result')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use 2 (not 1) spaces for indentation.

@@ -33,6 +33,11 @@ def with_scope(locator)
page.driver.browser.url_whitelist = ['http://127.0.0.1']
end

When /^I type in "([^"]*)" with "([^"]*)"$/ do |selector,contents|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space missing after comma.

@@ -36,7 +36,9 @@ def pseud
private
def tag_output(search_param, tag_type)
tags = Tag.autocomplete_lookup(:search_param => search_param, :autocomplete_prefix => "autocomplete_tag_#{tag_type}")
render_output tags.map {|r| Tag.name_from_autocomplete(r)}
output = tags.map {|r| Tag.name_from_autocomplete(r)}
Rails.cache.write('/v1/autocomplete_hack/last_result', output ) if ENV["RAILS_ENV"] == "test"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space inside parentheses detected.

@@ -36,7 +36,9 @@ def pseud
private
def tag_output(search_param, tag_type)
tags = Tag.autocomplete_lookup(:search_param => search_param, :autocomplete_prefix => "autocomplete_tag_#{tag_type}")
render_output tags.map {|r| Tag.name_from_autocomplete(r)}
output = tags.map {|r| Tag.name_from_autocomplete(r)}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space between { and | missing.
Space missing inside }.

@sarken
Copy link
Member

sarken commented Nov 10, 2016

Is this still a WIP?

@sarken
Copy link
Member

sarken commented Nov 26, 2016

This has conflicts and will need to be updated, but it's still marked "WIP," so I'm not sure if it's done.

@@ -303,6 +325,7 @@ def with_scope(locator)
sleep 120
end


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line detected.

# resist the temptation. You should only verify outcome that is observable for
# the user (or external system) and databases usually are not.
result = false
(1..20).each do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use Integer#times for a simple loop which iterates a fixed number of times.

@@ -33,6 +33,11 @@ def with_scope(locator)
page.driver.browser.url_whitelist = ['http://127.0.0.1']
end

When /^I type in "([^"]*)" with "([^"]*)"$/ do |selector,contents|
Rails.cache.delete('/v1/autocomplete_hack/last_result')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use 2 (not 1) spaces for indentation.

@@ -33,6 +33,11 @@ def with_scope(locator)
page.driver.browser.url_whitelist = ['http://127.0.0.1']
end

When /^I type in "([^"]*)" with "([^"]*)"$/ do |selector,contents|

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space missing after comma.

@@ -36,7 +36,9 @@ def pseud
private
def tag_output(search_param, tag_type)
tags = Tag.autocomplete_lookup(:search_param => search_param, :autocomplete_prefix => "autocomplete_tag_#{tag_type}")
render_output tags.map {|r| Tag.name_from_autocomplete(r)}
output = tags.map {|r| Tag.name_from_autocomplete(r)}
Rails.cache.write('/v1/autocomplete_hack/last_result', output ) if ENV["RAILS_ENV"] == "test"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space inside parentheses detected.

@@ -36,7 +36,9 @@ def pseud
private
def tag_output(search_param, tag_type)
tags = Tag.autocomplete_lookup(:search_param => search_param, :autocomplete_prefix => "autocomplete_tag_#{tag_type}")
render_output tags.map {|r| Tag.name_from_autocomplete(r)}
output = tags.map {|r| Tag.name_from_autocomplete(r)}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space between { and | missing.
Space missing inside }.

@zz9pzza zz9pzza changed the title [WIP} AO3-4702 The autocomplete work [WIP] AO3-4702 The autocomplete work Nov 26, 2016
@zz9pzza zz9pzza changed the title [WIP] AO3-4702 The autocomplete work AO3-4702 The autocomplete work Nov 26, 2016
@zz9pzza zz9pzza changed the title AO3-4702 The autocomplete work AO3-4702 Test autocomplete Nov 26, 2016
@shalott
Copy link
Member

shalott commented Nov 28, 2016

I hate to say this, but I think this is fundamentally a bad idea as well as a hack. You are not actually testing the autocomplete, you're just creating the illusion of having tested it, which IMO is worse because it gives us false confidence.

If you've got the testing-with-javascript working, I suspect the issue is you were not looking at the right field -- the autocomplete javascript does a thing where it replaces one div with another which holds the actual results. I actually wrote code that I think was working to access it but never managed to finish implementing the PR involved, I will dig the code out if you want?

@zz9pzza
Copy link
Contributor Author

zz9pzza commented Nov 30, 2016

I had high hopes for:

assert page.should have_css('div.autocomplete.dropdown', :text => answer)

but no

@sarken
Copy link
Member

sarken commented Apr 24, 2017

Closing in favor of #2877

@sarken sarken closed this Apr 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants