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

added feature_spec_helper to turn on javascript driver, added test fo… #132

Closed
wants to merge 1 commit into from

Conversation

dlinch
Copy link

@dlinch dlinch commented Oct 17, 2017

…r pledge button opening up a new tab

Resolves #131

Make sure you've covered the following bases in your PR. They won't all be
needed for every PR (ex. fixing a straightforward rubocop issue might have no
tradeoffs and add no dependencies).

  • What were the main changes/goals of this PR?
    To add javascript enabled feature tests, and test the pledge to donate button opening up on a new tab.

  • Did you need to make any tradeoffs? Why did you make the choice you made?
    it opens up amazon.com in the next tab. this should still work even if you can't connect to the internet.

  • Did you add any dependencies?
    no, selenium was already a dependency.

  • Anything else we should know about?

@@ -1,4 +1,4 @@
require "rails_helper"
require "features_helper"
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you forget to commit this file? It doesn't exist in the project.

https://travis-ci.org/rubyforgood/playtime/builds/288863689

Copy link
Collaborator

@leesharma leesharma left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I'll second filipewl's review (looks like you're missing a file), but I've got something else too:

I'm not thrilled with the idea of using another file instead of rails_helper for specific feature tests; let's keep it to one root configuration file. Could you move your configuration to a spec support file? This will involve creating a support file (ex. spec/support/capybara.rb) and importing it in spec/rails_helper.rb.

@leesharma
Copy link
Collaborator

Are you still working on this issue?

@leesharma
Copy link
Collaborator

I'm going to close this PR and free the issue. If you're still working on this, comment and we can re-open it.

@leesharma leesharma closed this Oct 23, 2017
@dlinch
Copy link
Author

dlinch commented Oct 24, 2017

I'll work on it this evening, I was called into jury duty last week and it went right up until a 4 day vacation so I was offline for a while.

@leesharma
Copy link
Collaborator

Sure thing–I hope you enjoyed your vacation! I'll reopen the PR.

@leesharma leesharma reopened this Oct 24, 2017
@leesharma leesharma added the WIP label Oct 24, 2017
@micahbales micahbales temporarily deployed to project-playtime-stagin-pr-132 November 21, 2017 03:33 Inactive
@leesharma
Copy link
Collaborator

I'm going to close this PR since it's been unchanged for a few months. Let me know if it should be reopened!

@leesharma leesharma closed this Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add JS integration test for "pledging opens a new Amazon tab"
4 participants