codewars / codewars.com

Issue tracker for Codewars
https://www.codewars.com
BSD 2-Clause "Simplified" License
2.09k stars 220 forks source link

Test suite: Hidden dependencies on native objects/functions/methods #941

Closed Voileexperiments closed 5 years ago

Voileexperiments commented 7 years ago

So, as I've documented comprehensively in #940, for some broken JS katas it's caused by Test suite implicitly depending on some JS native functions, which doesn't make any sense. Here is a list of hidden dependencies from what I can remember:

and who knows how many more dependencies exists. They cause problems with writing handicap katas because in the process of removing native objects/functions/methods, the test suite will malfunction, resulting in a sad, broken kata.

I think there are two ways to remove the dependency:

  1. Store the native dependencies internally first, then call the stored instances instead (assuming Test suite is initialized before preloaded code).
  2. Polyfill. Too big of an effort, and not always possible, but will work even if test suite is initialized after user code.

(I'm not sure if other languages have similar problems; haven't dug into those languages deeply to stumble on these kind of problems.)

Voileexperiments commented 7 years ago

I just discovered a very bad usage of this issue:

If any kata uses Test.assertSimilar for the tests, because for some reasons it uses JSON.stringify even for primitive data types e.g strings, I can always override JSON.stringify so anything would be equal to each other:

JSON.stringify=()=>''; //bad bad bad

I've read the cli framework codebase somewhat yesterday, and since Test methods are already cached and defined before any code, why not things that these methods depend on too?

kazk commented 7 years ago

why not things that these methods depend on too?

Because resources are limited and there's no end to this. Anyone can study the code like you did and come up with some workarounds. Most languages have some ways to cheat.


They cause problems with writing handicap katas because in the process of removing native objects/functions/methods, the test suite will malfunction, resulting in a sad, broken kata.

/home/codewarrior/solution.txt (or /workspace/solution.txt) should be read instead of messing with the builtins. The problem is lack of documentation and inability to edit broken kata. I can't do anything about the latter, but the new documentation contains this info.

Voileexperiments commented 7 years ago

Because resources are limited and there's no end to this.

I was thinking along the line of how lodash handles native method references, e.g _.random still works the same even if Math.random is hacked before or after the require. That line was referring to the fact that cw-2 is also a require in cli, so it can as well make local references to native methods to prevent native method overrides changing its behaviour.

relevant source code for lodash's random method

Incidentally, is it possible to inject cw-2 at the fixture instead of before preloaded? After all, the user shouldn't have access to anything related to the test suite in their user code. global and Test are restored at the point between user code and fixture anyway, so I think not injecting cw-2 before this point might be possible?

kazk commented 7 years ago

Just to be clear, by "resources are limited" I meant only few people actively contributes to that project and we can't do everything. It's just not realistic to keep patching for every cheat/workaround found. Anyone can open a PR suggesting fixes though.

Injecting at the fixture sounds like a good idea and should be possible by refactoring parts of cw-2.js. Note that cw-2.js is really old and full of legacy code. It's not even a proper module :(

I've been wanting to do something about cw-2.js, but it hasn't been my priority.

kazk commented 5 years ago

Closing as wontfix