Freecodecamp: [Arguments Optional] Current test cases allow for unintended solutions to pass

[colinthornton]

Describe your problem and how to reproduce it:
On the Intermediate Algorithm Scripting: Arguments Optional challenge, the only two happy path test cases -- addTogether(2, 3) and addTogether(2)(3), both expect a return value of 5. All other test cases expect a return value of undefined. Because there are no other happy path test cases, it's possible to construct a passing solution that doesn't add inputs together, but simply returns 5 when the inputs are valid numbers (pasted below). Another set of happy path test cases could help prevent this sort of unintended solution from passing.

// passes tests without performing addition
function addTogether(x, y) {
  if (y) {
    return returnFive(x, y);
  }
  return curriedReturnFive(x);
}

function returnFive(x, y) {
  if (!areNumbers(x, y)) {
    return;
  }
  return 5; // just 5 satisfies current tests
}

function curriedReturnFive(x) {
  if (!isNumber(x)) {
    return;
  }
  return function(y) {
    return returnFive(x, y);
  }
}

function areNumbers(...args) {
  return args.every(isNumber);
}

function isNumber(x) {
  return typeof x === 'number';
}

Add a Link to the page with the problem:
https://www.freecodecamp.org/learn/javascript-algorithms-and-data-structures/intermediate-algorithm-scripting/arguments-optional

If possible, add a screenshot here (you can drag and drop, png, jpg, gif, etc. in this box):

[Sky020]

Do you not think this is a serious _edge-case_? Seems like going through more trouble to trick the tests, than to write the correct solution.

[colinthornton]

I haven't done anything to trick the tests -- the only change I would make to my above code would be changing return 5 to return x + y, everything else is input validation which would be nearly the same in anyone's solution. You could take any of the Get a Hint solutions and replace the actual addition with 5 and would get the same results.

Do I think that many people would actually write their solution like this? No. But I don't think it would hurt to add another couple of happy path tests so that it's not even an option.

[Sky020]

You are right, it would not hurt to add an extra test. I do it is of little consequence, though; the users still need to do every aspect of the lesson (return a function based on input arguments, and perform checks on the arguments).

Let us wait for one more opinion on moving forward with this issue. If agreed, then I suggest we open this to first-timers.

[lasjorg]

@colinthornton How do you suggest we test that the arguments passed in are actually being added together before returning the value? The seed code and the requirements are so nonspecific that we really do not have much to go on.

If we were forcing the use of a predefined function definition that had two parameters we would be able to check that they got added together, but we are allowing a function with, no parameters, spread syntax, or an unspecified number of parameters.

[moT01]

Lol, why does one of the tests link to an 80's music video? I'm not sure if we need that in there, but it did give me a chuckle.

We could just add another test that checks different values?
addTogether(5,6) should return 11 - not sure if that proves addition was used because they could just make a conditional that checks if(first === 2) return 5 else return 11. But many of the challenges are like this. I don't mind adding another test, but for the most part - something like this falls under the honesty policy I think. Users who hack the tests, aren't completing the challenge properly - which I could argue, might be more difficult and beneficial in some cases. But adding one more tests that checks different values would ensure that the function works for more than one case.

[lasjorg]

Lol, why does one of the tests link to an 80's music video? I'm not sure if we need that in there, but it did give me a chuckle.

It's a rickroll (maybe you did know that and just wanted to rickroll?)

We could just add another test that checks different values?

Sounds like a plan, for some reason I didn't even think of that.

And like you said, we can't test for every way a camper might circumvent the tests and it shouldn't be a priority

[Twaha-Rahman]

I'm working on adding another test ASP! ;)

[Sky020]

@Twaha-Rahman , it is great that you want to help, but seeing as you have contributed before, we are going to prioritise a PR from a _first timer_.

[Twaha-Rahman]

@Sky020 Oh...sorry! Half-way through it, should I proceed or abort?

[Twaha-Rahman]

Sorry for the inconvenience! 😅😅

[Sky020]

@Twaha-Rahman , I suggest you abort. Feel free to work on any issues with the label _help wanted_ and not _first timers welcome_, though. 👍

[Twaha-Rahman]

Thanks! ;)

[rajat641]

@Sky020 - Please allow me to work on this. I am new to open source and it would be great If I can work on this .

[ieahleen]

@rajat641 : go for it, there is no need to ask for permission to work on something in this repo if there is consensus in the issue about what change to do.

[rajat641]

@Ieahleen - Thanks a lot. I asked for permission so as to ensure that I'm only working on an issue.

[rajat641]

Hi @Ieahleen @moT01 @Sky020 , I hope you're good. I have raised a PR for this Issue with adding 2 extra test cases. Let me know if more test cases are to be added or something is not right. Thanks a lot guys for the opportunity.