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

Convert InputMixin to a high-order Component #1775

Merged

Conversation

psinghal20
Copy link
Contributor

Fixes #1456.
It replaces the InputMixin with InputHOC component which is then used with several input fields to validate entries by using validation entries.

@psinghal20
Copy link
Contributor Author

@ragesoss I separated the InputMixin conversion to another PR as it was getting harder to find and fix the errors with changes done to the validation store. I will continue with ValidationStore changes in another PR.

@psinghal20
Copy link
Contributor Author

@ragesoss Is the failing Travis build a random failure? I don't think I made any changes to CourseCloneModal. If it is a random failure, can you please restart the Travis build.

@ragesoss
Copy link
Member

Will do. Can you confirm that that spec passes locally? It does some unique things in terms of state, inputs, and validations, and this one is not one that I've seen fail randomly in a long while.

@ragesoss
Copy link
Member

@psinghal20 same spec failed again. It's probably a real failure.

@AmitJoki
Copy link
Contributor

Have you tried checking which input components use InputHOC in the cloning process? That might help you in singling out the problem.

@psinghal20
Copy link
Contributor Author

@ragesoss The specs seems to fail when the same course term is used as the original course, but in my local branch, the failure message does appear when the course term is same as the original course. When I try to run this spec on my local branch, the test fails and raises the error PhantomJS client died while processing. I am not sure about the error on my local branch and in case of Travis build, as the message does appear on my local, I think updating the spec with a sleep statement like sleep 2 maybe can help with the failing spec.

@ragesoss
Copy link
Member

I take it that course cloning works as expected when you do it manually? Putting in the same term as the original will show the error message and let you change them term and proceed?

@ragesoss
Copy link
Member

expect(page).to have_content('This course already exists')

That uses Capybara's wait behavior, so it will wait up to 15 seconds for the expectation to be met before timing out. Sleep is unlikely to help.

@psinghal20
Copy link
Contributor Author

Yes, it does behave as expected when I try it manually but I don't know why the local spec makes my PhantomJS client die.

@ragesoss
Copy link
Member

Okay. I'll poke at it next week.

@psinghal20
Copy link
Contributor Author

@ragesoss Found what was causing my local failure, it was because of console statement I added to debug. Removed and ran all the feature specs. All specs pass on my local build.

@ragesoss ragesoss merged commit f43f603 into WikiEducationFoundation:master Mar 13, 2018
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.

3 participants