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

Pass 2 over testing guide #23208

Merged
merged 1 commit into from
Jan 25, 2016
Merged

Conversation

vipulnsward
Copy link
Member

  • Grammar fixes
  • Wordsmitting
  • Fixed wrong statement about association usage in fixtures
  • Changed association name from 'one' to 'first' instead
  • More consistent usage of we/our
  • Mentions assert_select is below, not already covered in Integration test.

[ci skip]

@rails-bot
Copy link

r? @schneems

(@rails-bot has picked a reviewer for you, use r? to override)

@@ -589,9 +589,9 @@ class BlogFlowTest < ActionDispatch::IntegrationTest
end
```

If you remember from earlier in the "Testing Views" section we covered `assert_select` to query the resulting HTML of a request.
We will take a look at `assert_select` to query the resulting HTML of a request in the "Testing Views" section below. It is used for testing the response of our request by asserting the presence of key HTML elements and their content.
Copy link
Member Author

Choose a reason for hiding this comment

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

Question: Is this section out of order? We were referring to "Testing Views" which is defined later.
I think, we can move integration tests below others.

cc @zzak

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, we should leave it. To me, at least, it fits with the flow of the section it's apart of.

@maclover7 maclover7 added the docs label Jan 23, 2016
@@ -546,17 +546,17 @@ class UserFlowsTest < ActionDispatch::IntegrationTest
end
```

Inheriting from `ActionDispatch::IntegrationTest` comes with some advantages. This makes available some additional helpers to use in your integration tests.
Here the test is inheriting from `ActionDispatch::IntegrationTest`. This makes available some additional helpers to use in our integration tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes some additional helpers available for us to use in our integration tests.

@maclover7
Copy link
Contributor

So far, looks fine to me, with a few comments.

- Grammar fixes
- Wordsmitting
- Fixed wrong statement about association usage in fixtures
- Changed association name from 'one' to 'first' instead
- More consistent usage of we/our
- Mentions assert_select is below, not already covered in Integration test.

[ci skip]
@vipulnsward
Copy link
Member Author

@maclover7 updated. The above question remains. Can handle in other request after confirmation.

@vipulnsward vipulnsward assigned maclover7 and unassigned schneems Jan 25, 2016
@maclover7
Copy link
Contributor

👍 Thank you for your contribution!

maclover7 added a commit that referenced this pull request Jan 25, 2016
@maclover7 maclover7 merged commit ceea381 into rails:master Jan 25, 2016
@maclover7
Copy link
Contributor

❤️ 💚 💙 💛 💜

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.

None yet

4 participants