-
Notifications
You must be signed in to change notification settings - Fork 219
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
Recent Changes 2019-02-xx #1726
Comments
@kazk Since the new Node version adds native |
Maybe add something like: if (!/^v[68]\./.test(process.version)) throw new Error("Only Node 6/8"); to prevent making it available on Node 10+. Removing a language from a kata is not handled well and I don't think it's worth the risk. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Don't know where to report this, so will use this thread.
Apparently, |
^ Chai has acknowledged the issue a few months ago but it's on the roadmap to Chai 5, which means it'll probably take a year or two before an official fix will see the light of day. @kazk There are lots of other issues with Chai:
Array.prototype.foo = 'bar';
describe("Solution", function() {
it("should test for something", function() {
Test.assertEquals([2,4,6,8], [2,4,6,9]);
});
}); In Node 8 it outputs Moreover, util.inspect behaviour can be customized while I don't see how to do this with Chai.
Can we just use the old |
Chai is one of the most commonly used assertion libraries and we haven't had any complaints like that from users on Qualified. That being said, we don't need to use Chai if it's problematic. We can install other assertion library if there's anything better.
Can you elaborate on this? I don't understand what you mean by "it defeats the purpose of using a testing framework".
Old |
For BigInt, it's just not in the current proposal yet so it can't be implemented. Remember that BigInt is still in proposal (even it's in stage 3). See https://github.com/tc39/proposal-bigint |
What I mean is if the testing framework provides things like That is, yes, Also, |
Other comments: I'm looking at the current list of installed modules in the Node 10.x environment:
|
It does work as expected. It's just that output message is different from cw-2 and
Some packages exists for Qualified. |
|
One more thing: Chai's inspect behaves differently from before when proxies are involved, e.g:
Node 8 says Things will only get even worse once |
I just tried and got different output. Node 8:
Node 10:
How is the deep equality of proxied object should be defined? The output of Node 10 looks correct to me because the |
This is massively complicated once
Trying to enumerate proxies for display purposes is a very bad idea in general because of how easy it can break horribly. Maybe use |
Why should we display like that on test failure for deep equality? When testing objects with If an object needs to be a Proxy that behaves in a certain way, then it should be tested accordingly and not using |
Because proxies can reference itself and turn into deep proxies, which makes for a potentially very large, unbounded structure. Proxies are lazy, and so the display should be lazy too, or at least don't go out of bounds easily. With proxies these things can happen:
That is, it's not sensible to expand the full content of a proxied object because they can do pretty much anything. Of course, In addition to this, there are more issues:
|
Also,
BigInt proposal is still in stage 3 because to move a proposal from stage 3 to stage 4 there must be "two implementations by two independent VMs and they must pass the acceptance tests". So I don't see why it can't be implemented; that'd be circular reasoning. |
I'll read the previous comment tomorrow, but for BigInt one, I'm talking about its |
We can have assertions and utility package for Codewars that the community can maintain like If something that handles Proxy better is needed, we can add it there.
I wrote how to disable this in the template and also in wiki. Set
That was part of rendering Chai in cw-2.js. Node 10 Codewars doesn't use Can you open a new issue on the runner repo summarizing what the community wants? We can continue there, this is starting to get little off topic. |
Changes since 2018-12-xx. I haven't been able to work on Codewars recently so it's mostly Code Runner changes.
Feel free to post comments here if you have any questions about the changes.
Code Runner
New Languages ✨
New Language Versions
rand
needs to be updated.Other Updates
Test.Hspec.Codewars
by @Bubbler-4Codewars
describe/it
properlyit
)The text was updated successfully, but these errors were encountered: