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

Fix spec failing due to duplicate component #734

Conversation

hrishimittal
Copy link
Contributor

@hrishimittal hrishimittal commented Feb 23, 2017

On running rake, the following spec fails:

  1) Pages/Index All in one page Simple Component Without Redux changes name in message according to input
     Failure/Error:
       within(dom_selector) do
         find("input").set new_text
         within("h3") do
           is_expected.to have_content new_text
         end
       end

     Capybara::Ambiguous:
       Ambiguous match, found 2 elements matching css "div#HelloWorld-react-component-4"
     Shared Example Group: "React Component" called from ./spec/features/integration_spec.rb:47

The reason is a duplicate component with the id HelloWorld-react-component-4 in pages/index.html.erb.


This change is Reviewable

hrishimittal added a commit to hrishimittal/react_on_rails that referenced this pull request Feb 23, 2017
@coveralls
Copy link

coveralls commented Feb 23, 2017

Coverage Status

Coverage remained the same at 99.329% when pulling 2d3f112 on hrishimittal:fix-spec-duplicate-helloworld-component into 3106b4b on shakacode:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.329% when pulling 2d3f112 on hrishimittal:fix-spec-duplicate-helloworld-component into 3106b4b on shakacode:master.

@coveralls
Copy link

coveralls commented Feb 23, 2017

Coverage Status

Coverage remained the same at 99.329% when pulling 2d3f112 on hrishimittal:fix-spec-duplicate-helloworld-component into 3106b4b on shakacode:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.329% when pulling 2d3f112 on hrishimittal:fix-spec-duplicate-helloworld-component into 3106b4b on shakacode:master.

@hrishimittal
Copy link
Contributor Author

Travis failed because of a timeout. Can someone please re-run it? - https://travis-ci.org/shakacode/react_on_rails/jobs/204543891#L3795

@justin808
Copy link
Member

This change does not look valid. Can we close this PR?


Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


spec/dummy/app/views/pages/index.html.erb, line 77 at r1 (raw file):

  <%%= react_component("HelloES5", props: @app_props_hello, prerender: false, trace: true, id: "HelloES5-react-component-5") %>
</pre>
<%= react_component("RenderedHtml", prerender: true, trace: true, id: "HelloWorld-react-component-4") %>

you essentially took out the running of this test of generating some html on the server.

the code directly above is in a pre block and it's not rendered per the <%%


Comments from Reviewable

@wanyamaman
Copy link
Contributor

I was also looking at this.

Thoughts: I suspect the solution is to increase the id number and write corresponding tests for it. There might also be a missing component since the pre tag shows two components but the actual code has one.

@hrishimittal
Copy link
Contributor Author

@justin808 I understand the pre block code is not rendered. But look at this line -

https://github.com/shakacode/react_on_rails/pull/734/files#diff-13675373e6eaa2b6e91995c3ec8974f7R84

The same id is used again, which is why the spec was failing.

Removing the RenderedHtml line didn't break any test, so that suggests maybe a test is missing?

As @wanyamaman suggested, we probably need another id and corresponding tests.

@justin808
Copy link
Member

@hrishimittal The ids shouldn't conflict. Can you put back the removed line and just change around the ids?

@justin808
Copy link
Member

Removing the RenderedHtml line didn't break any test, so that suggests maybe a test is missing?
and yes! looks that way!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.329% when pulling 13b35c7 on hrishimittal:fix-spec-duplicate-helloworld-component into 3106b4b on shakacode:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.329% when pulling 13b35c7 on hrishimittal:fix-spec-duplicate-helloworld-component into 3106b4b on shakacode:master.

@coveralls
Copy link

coveralls commented Feb 23, 2017

Coverage Status

Coverage remained the same at 99.329% when pulling 13b35c7 on hrishimittal:fix-spec-duplicate-helloworld-component into 3106b4b on shakacode:master.

@hrishimittal
Copy link
Contributor Author

@justin808 I've restored the deleted line and changed ids to fix the spec. If you can give me a LGTM, I'll squash and rebase.

@justin808
Copy link
Member

@hrishimittal Please rebase on top of master and push force. Thanks!

On running `rake`, the following spec fails:

```
  1) Pages/Index All in one page Simple Component Without Redux changes name in message according to input
     Failure/Error:
       within(dom_selector) do
         find("input").set new_text
         within("h3") do
           is_expected.to have_content new_text
         end
       end

     Capybara::Ambiguous:
       Ambiguous match, found 2 elements matching css "div#HelloWorld-react-component-4"
     Shared Example Group: "React Component" called from ./spec/features/integration_spec.rb:47
```

The reason is a duplicate component with the id `HelloWorld-react-component-4` in `pages/index.html.erb`.

Fix by bumping id number.

Add entry for PR shakacode#734 to Changelog.
@hrishimittal hrishimittal force-pushed the fix-spec-duplicate-helloworld-component branch from 13b35c7 to 07d2746 Compare February 24, 2017 23:52
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.329% when pulling 07d2746 on hrishimittal:fix-spec-duplicate-helloworld-component into 4943fbd on shakacode:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.329% when pulling 07d2746 on hrishimittal:fix-spec-duplicate-helloworld-component into 4943fbd on shakacode:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.329% when pulling 07d2746 on hrishimittal:fix-spec-duplicate-helloworld-component into 4943fbd on shakacode:master.

@justin808 justin808 merged commit 1e06286 into shakacode:master Feb 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants