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(curriculum): added test to allow any order solution #38771

Conversation

ShaunSHamilton
Copy link
Member

Checklist:

  • I have read freeCodeCamp's contribution guidelines.
  • My pull request has a descriptive title (not a vague title like Update index.md)
  • My pull request targets the master branch of freeCodeCamp.
  • All the files I changed are in the same world language, for example: only English changes, or only Chinese changes, etc.

Closes #38769

Changed test to include all solution orders. Added test to prevent simple cheating.

@gitpod-io
Copy link

gitpod-io bot commented May 8, 2020

@camperbot camperbot added language: English scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory. labels May 9, 2020
@ilenia-magoni
Copy link
Contributor

Sorry, it seems I did something wrong... I am starting to learn how this works.

@ShaunSHamilton
Copy link
Member Author

@ieahleen , no worries. I appreciate the extra approval 😃

Copy link
Member

@moT01 moT01 left a comment

Choose a reason for hiding this comment

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

Do you think someone might have those numbers (from the new test) in their code as comments @SKY020? Preventing them from passing. Probably not, I guess. You could use the removeJSComments thing to ensure it doesn't cause a problem.

Side note - almost all of the Euler problems can be passed by simply returning the expected result and not creating any algorithm. Wonder if we should add a quick test for many of the others that check for something similar? Maybe an issue is in order for that.

@ShaunSHamilton
Copy link
Member Author

@moT01, yes, the comments could be a problem, for some. I will look into adding the removeJSComments, as we do get some forum posts of users not passing because of comments.

Maybe an issue is in order for that.

Sure, let us get that up so we can list the culprit challenges.

Copy link
Member

@moT01 moT01 left a comment

Choose a reason for hiding this comment

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

LGTM @SKY020 🎉

I like the extra test for checking that users aren't directly returning the expected results. But I'm having second thoughts about how easy it is to get around it, and wondering if it's worth it. It's fine for this one, better than nothing. But it's pretty easy to get around, and I was thinking more about the rest of the Euler challenges that we were talking about.

@ShaunSHamilton
Copy link
Member Author

@moT01 , I cannot think of a decent method to test for cheating without some sort of comparison to the/a solution. That is, looking for the use of specific methods...

@ojeytonwilliams
Copy link
Contributor

Given that this is intended to help with interviews, people really would be shooting themselves in the foot if they cheat. It seems fine to use simple checks like this, but ultimately we can't stop users, say, copying other people's solutions.

Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ojeytonwilliams ojeytonwilliams merged commit 9ad4c98 into freeCodeCamp:master May 18, 2020
@ShaunSHamilton ShaunSHamilton deleted the fix/add-test-euler-problem-43 branch June 20, 2020 22:30
abbathaw pushed a commit to abbathaw/freeCodeCamp that referenced this pull request Jul 24, 2020
…#38771)

* added test to allow any order solution

* add removeJSComments for tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: curriculum Lessons, Challenges, Projects and other Curricular Content in curriculum directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

for Euler Problem 43 - numbers must be in a certain order to pass the tests (but it is not specified)
5 participants