Skip to content

Fix spec failing due to duplicate component#734

Merged
justin808 merged 1 commit intoshakacode:masterfrom
hrishimittal:fix-spec-duplicate-helloworld-component
Feb 25, 2017
Merged

Fix spec failing due to duplicate component#734
justin808 merged 1 commit intoshakacode:masterfrom
hrishimittal:fix-spec-duplicate-helloworld-component

Conversation

@hrishimittal
Copy link
Copy Markdown
Contributor

@hrishimittal hrishimittal commented Feb 23, 2017 •
edited by justin808
Loading

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
@hrishimittal
Add entry for PR shakacode#734 to Changelog
2d3f112
@coveralls
Copy link
Copy Markdown

coveralls commented Feb 23, 2017 •
edited
Loading

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
Copy Markdown

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.

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 23, 2017 •
edited
Loading

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
Copy Markdown

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.

@hrishimittal
Copy link
Copy Markdown
Contributor Author

hrishimittal commented Feb 23, 2017

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

@hrishimittal hrishimittal mentioned this pull request Feb 23, 2017
Merged
@justin808
Copy link
Copy Markdown
Member

justin808 commented Feb 23, 2017

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
Copy Markdown
Contributor

wanyamaman commented Feb 23, 2017

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
Copy Markdown
Contributor Author

hrishimittal commented Feb 23, 2017

@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
Copy Markdown
Member

justin808 commented Feb 23, 2017

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

@justin808
Copy link
Copy Markdown
Member

justin808 commented Feb 23, 2017

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
Copy Markdown

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.

2 similar comments
@coveralls
Copy link
Copy Markdown

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.

@coveralls
Copy link
Copy Markdown

coveralls commented Feb 23, 2017 •
edited
Loading

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
Copy Markdown
Contributor Author

hrishimittal commented Feb 24, 2017

@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
Copy Markdown
Member

justin808 commented Feb 24, 2017

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

@hrishimittal
Fix spec failing due to duplicate component
07d2746 
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
Copy Markdown

coveralls commented Feb 25, 2017

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
Copy Markdown

coveralls commented Feb 25, 2017

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
Copy Markdown

coveralls commented Feb 25, 2017

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
@ihabadham ihabadham mentioned this pull request Feb 14, 2026
Closed
@claude claude Bot mentioned this pull request Feb 26, 2026
Merged
3 tasks
This was referenced Mar 8, 2026
Merged
Merged
Merged
Merged
Merged
This was referenced Mar 10, 2026
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Mar 17, 2026
Merged
Merged
Merged
This was referenced Mar 26, 2026
Merged
Merged
Closed
Merged
Merged
Merged
This was referenced Apr 18, 2026
Merged
Merged
Merged
This was referenced May 4, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hrishimittal @coveralls @justin808 @wanyamaman