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 test to fail on incorrect input #5007

Conversation

LenaBarinova
Copy link
Contributor

In 'Waypoint: Add Font Awesome Icons to our Buttons' challenge changed a test to check whether element is within Like button (did it the same way as it is done in 'Waypoint: Add Font Awesome Icons to all of our Buttons' challenge).
closes #4981

@SaintPeter
Copy link
Member

Don't you need to do the prior test as well as the new test? It looks like you're testing to make sure it's in a button, but now it could be any button, not just the like button.

@LenaBarinova
Copy link
Contributor Author

I'm checking the button of a class .btn-primary, which is our like button's class.

@SaintPeter
Copy link
Member

Yes, but the example was someone creating a whole new button. If they could do that, they could create a copy of the button's classes as well.

Maybe I'm being over sensitive. @ltegman, @bugron ?

@bugron
Copy link
Contributor

bugron commented Dec 4, 2015

Agreed with @SaintPeter. We need both of these tests. You can combine them: $(\"i.fa-thumbs-up\").parent().text().match(/Like/gi) && $(\".btn-primary > i\").hasClass(\"fa fa-thumbs-up\") or just left first test case unchanged and add second one with $(\".btn-primary > i\").hasClass(\"fa fa-thumbs-up\"). Being more precise is better.

@bugron
Copy link
Contributor

bugron commented Dec 4, 2015

@LenaBarinova OK, but now your second test checks for the Link text too. Remove $(\"i.fa-thumbs-up\").parent().text().match(/Like/gi) && from your second test, then run these commands:

git add .
git commit --amend

and then force push your branch to your fork: git push -f YOUR_ORIGIN YOUR_BRANCH_NAME.
You can read more about this in this article: How To Create A Pull Request for Free Code Camp.

@LenaBarinova
Copy link
Contributor Author

I've appended the test, didn't create new one, since we are checking the same logical thing - fa-thumbs-up icon should be located within the Like button.

@bugron
Copy link
Contributor

bugron commented Dec 4, 2015

@LenaBarinova oh, yes, sorry, my fault.

…ns to our Buttons

Added more precise checking for not only class of the button but also text
@bugron
Copy link
Contributor

bugron commented Dec 4, 2015

@LenaBarinova I've verified that this works. After you squash your commits into one I'm ready to merge your PR.
Update: @LenaBarinova thanks a lot, verified and merged!

@LenaBarinova LenaBarinova force-pushed the fix/incorrect-input-still-passes-the-test branch from 09343f0 to 0b48d7e Compare December 4, 2015 06:53
bugron added a commit that referenced this pull request Dec 4, 2015
…passes-the-test

Fix test to fail on incorrect input
@bugron bugron merged commit b5df6ca into freeCodeCamp:staging Dec 4, 2015
@LenaBarinova
Copy link
Contributor Author

@bugron all done!

@LenaBarinova
Copy link
Contributor Author

@bugron Thanks for the help! It was first commit, I hope further commits will go more smoothly :)

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.

None yet

3 participants