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

Add CoffeeScript 2.7.0 #277

Open
KayleighWasTaken opened this issue Dec 28, 2023 · 9 comments
Open

Add CoffeeScript 2.7.0 #277

KayleighWasTaken opened this issue Dec 28, 2023 · 9 comments

Comments

@KayleighWasTaken
Copy link

KayleighWasTaken commented Dec 28, 2023

  • Name: CoffeeScript
  • Version: 2.7.0
  • Release Note/Changelog:
    Changelogs can be found at coffeescript.org.

The main rationale behind requesting this update is due to the extremely outdated (and slow) Node version (v 8.7.0) that the current CS 1.10.0 runner is using. This will also make transitioning away from the Codewars test framework to the chai framework used by the JS/TS runners easier, as chai assertions (and assertions in general) behave very poorly on Node v8, pictured below.

An assertion being run on the Node v8 JS/CS runner:
image

The same assertion being run on the Node v10 JS runner:
image

This is an issue as for Kata with large expected outputs, for example The Observed Pin, where the output buffer can become entirely filled without having chai truncate the test output and giving the solver unhelpful error messages such as expected [ Array(1440) ] to deeply equal [ Array(1440) ]. The performance of certain assertions also seems to be severely degraded on the current runner, as a single deep equality check between two ~30000 element arrays takes over 12000ms on its own.

@KayleighWasTaken
Copy link
Author

If updating to CS 2+ is currently out of scope, then simply switching over to the current latest JS Node runner (v18.x) will give significantly improved assertion behaviour and performance.

@KayleighWasTaken
Copy link
Author

KayleighWasTaken commented Dec 28, 2023

If updating to CS 2+ is currently out of scope, then simply switching over to the current latest JS Node runner (v18.x) will give significantly improved assertion behaviour and performance.

The actual performance improvement of the v18.x runner over the v8.7.0 runner is more than an order of magnitude when working with specific assertions. Using the compiled JS output of this translation in a kumite (with random test specifications changed to match other languages rather than the limited ones implemented for the Node v8 runner) gives a speedup from timeout to ~200ms runtime.

To verify my findings simply fork the kumite and run the tests with different Node runners selected.

@kazk
Copy link
Member

kazk commented Jan 3, 2024

This will also make transitioning away from the Codewars test framework to the chai framework

I know this is confusing, but Chai is an assertion package and not a test framework. Mocha is the test framework used in Node 10+ and TypeScript. It provides describe/it, and runs the tests. Chai is the most commonly used assertion package and also used by our package that provides some compatibility (Test.*) for Node 10+.

I'm sure the newer Node version is faster, but you can't really compare the performance of tests between Node v8 and v18 on Codewars because Node v8 is not using Mocha+Chai.

The performance of certain assertions also seems to be severely degraded on the current runner, as a single deep equality check between two ~30000 element arrays takes over 12000ms on its own.

Are you comparing the performance of Test.assertDeepEquals? I'd guess it's most likely because the custom test framework used in Node v8 and CoffeeScript uses lodash's isEqual under the hood instead of Chai. The Test.assertDeepEquals in Node 10+ is assert.deepEqual from Chai.

as chai assertions (and assertions in general) behave very poorly on Node v8, pictured below.

As I wrote in #233, the difference for the test output is the test framework. In Kumite, you can try Mocha with Node v8 by changing the test framework on the bottom right.


If we decide to add CoffeeScript 2, we'll use Mocha and the latest LTS Node.

@KayleighWasTaken
Copy link
Author

Are you comparing the performance of Test.assertDeepEquals? I'd guess it's most likely because the custom test framework used in Node v8 and CoffeeScript uses lodash's isEqual under the hood instead of Chai. The Test.assertDeepEquals in Node 10+ is assert.deepEqual from Chai.

I am comparing chai.assert.deepEqual in both versions, chai is an available package for the CS 1.10.0 runner. The performance of assert.deepEqual in v8/v10 vs v18 (all on mocha) also about an order of magnitude slower.

@kazk
Copy link
Member

kazk commented Jan 3, 2024

Cool. CoffeeScript has Chai v3.5.0. Node v12/v14/v18 has Chai v4.3.x.

@kazk
Copy link
Member

kazk commented Jan 3, 2024

deep equality has been rewritten from the ground up to support ES6 types like Map and Set, and better support existing types. It is now also much, much faster than before and allows us to bring some great improvements in upcoming releases.
https://github.com/chaijs/chai/releases/tag/4.0.0

@KayleighWasTaken
Copy link
Author

@kazk @ejini6969 was wondering when any potential updates to CS will commence and if there is anything from our side that can help (PRs or similar)? He says he's planning on updating all current CS kata to use Chai assertions this weekend as well, and is wondering if that is needed or it can be automated from your end?

@kazk
Copy link
Member

kazk commented Jan 5, 2024

To maximize future compatibility with Mocha, make sure to have the valid test structure and use nullary functions. See the list at the top of List of JavaScript Kata to Update for more details. We can provide assertions from Test.* that uses Chai with our package like JavaScript, and we can easily remove the Test. prefix from Test.describe/Test.it, but it's difficult to automatically fix the lack of test structure.

Also, make sure the tests are not using Node's assert that's imported implicitly. See how the current test file is created (#233 (comment)).

@ejini6969
Copy link

Was aware of those remarks! Currently using 1 and 2 as reference for update. Both have different test structures but compatible with the requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants