-
Notifications
You must be signed in to change notification settings - Fork 66
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
[TEST] Pcaston add tests for recaptcha service #599 #593
[TEST] Pcaston add tests for recaptcha service #599 #593
Conversation
…ests-for-recaptcha-service-AgileVentures#599
@@ -250,9 +250,9 @@ def stub_cruncher_match_jobs_retry_auth_fail | |||
.to_raise(RestClient::Unauthorized) | |||
end | |||
|
|||
def stub_cruncher_match_jobs_fail(result_code) | |||
def stub_cruncher_match_jobs_fail(resultCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use snake_case for variable names.
@@ -163,9 +163,9 @@ def stub_cruncher_job_create_retry_auth_fail | |||
.to_raise(RestClient::Unauthorized) | |||
end | |||
|
|||
def stub_cruncher_job_create_fail(result_code) | |||
def stub_cruncher_job_create_fail(resultCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use snake_case for variable names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to snake case
@@ -162,6 +162,7 @@ def search_text(text) | |||
end | |||
|
|||
And(/^show me the page$/) do | |||
save_and_open_page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debugger entry point save_and_open_page.
Then(/^I should be logged out$/) do | ||
cookies = page.driver.cookies | ||
expect(cookies['person_type']).to be nil | ||
expect(cookies['user_id']).to be nil | ||
end | ||
|
||
Given(/^I click the recaptcha$/) do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra empty line detected at block body beginning.
@@ -250,9 +250,9 @@ def stub_cruncher_match_jobs_retry_auth_fail | |||
.to_raise(RestClient::Unauthorized) | |||
end | |||
|
|||
def stub_cruncher_match_jobs_fail(result_code) | |||
def stub_cruncher_match_jobs_fail(resultCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use snake_case for variable names.
@@ -163,9 +163,9 @@ def stub_cruncher_job_create_retry_auth_fail | |||
.to_raise(RestClient::Unauthorized) | |||
end | |||
|
|||
def stub_cruncher_job_create_fail(result_code) | |||
def stub_cruncher_job_create_fail(resultCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use snake_case for variable names.
@@ -162,6 +162,7 @@ def search_text(text) | |||
end | |||
|
|||
And(/^show me the page$/) do | |||
save_and_open_page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove debugger entry point save_and_open_page.
Then(/^I should be logged out$/) do | ||
cookies = page.driver.cookies | ||
expect(cookies['person_type']).to be nil | ||
expect(cookies['user_id']).to be nil | ||
end | ||
|
||
Given(/^I click the recaptcha$/) do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra empty line detected at block body beginning.
user = User.find_by_first_name(first_name) | ||
expect(user.last_name).to eql last_name | ||
expect(user.phone).to eql phone | ||
Then(/^I\sshould\sverify\sthe\schange\sof\sfirst_name\s"(.*?)",\s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is OK but I don't think it is necessary and it makes the regex harder to read.
The pattern /text1 text2/
is treated exactly the same as /text1\stext2/
, so the addition of \s
is not needed.
true | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment above explains where I ended up with this step, after trying to simulate a user action (checking the box). The stub method did not work, but simply overriding the verify
method does the job.
@@ -162,6 +162,7 @@ def search_text(text) | |||
end | |||
|
|||
And(/^show me the page$/) do | |||
save_and_open_page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to have been dropped in a prior PR. Adding it back ....
app, | ||
browser: :chrome, | ||
args: ['headless'] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some restructuring to simplify, and also add Chrome as browser option for the Selenium web driver.
As prospective user of PETS | ||
I want to make contact and send a message | ||
|
||
@selenium_browser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to switch this test to use Selenium, driving a Chrome browser in "visible" mode (both poltergeist, and headless Chrome, failed to work).
I also have to use special recaptcha keys - these were added to the application.yml
file - will DM you on slack with that updated file.
See: https://developers.google.com/recaptcha/docs/faq (section on automated tests)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I forgot to mention that one of the problems you were having with using poltergeist was that phantomjs support for ES2015 (aka ES6) is still a work-in-progress. (ariya/phantomjs#14506).
(Poltergeist uses phantomjs to support js code).
@arun1595 is using ES6 in some of his code (e.g. promise
). Poltergeist silently fails when ES6 is encountered, so there is no indication of what caused the test to fail. Going to Chrome as the Selenium browser fixed this issue (but, for these tests, only when I used Chrome in visible (non-headless) mode.)
And I fill in "Message" with "Hi Metplus" | ||
And I click the recaptcha | ||
And I click the "Send Message" button | ||
Then I should see "Your message was sent successfully!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest also adding a "sad" path - that is, the user fills out the info but fails to check the "not a robot" box.
Test PR with changes to make all tests work.