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

Proposal: generalize exempting of tests requiring advanced V8 features #4996

Closed
rdeforest opened this issue Mar 1, 2018 · 4 comments
Closed

Comments

@rdeforest
Copy link
Contributor

Per discussion on #4893, tests which require switches be passed to node/coffee have their special handling hacked into Cakefile.

My proposal (and I'm already working on a PR) is to move special-case test files to their own directories under test/, accompanied by a config.json which contains the particulars of what's special about those tests.

(I figured it made more sense to open this than to continue off-topic discussion on the aforementioned PR.)

@rdeforest
Copy link
Contributor Author

The first problem I'm seeing is that in the current architecture the scanning of test/ happens after re-invocation of bin/cake with (or without) --harmony. I will probably have to make some relatively invasive changes to implement this suggestion.

@GeoffreyBooth
Copy link
Collaborator

I don’t think you need to worry about whether --harmony is invoked or not. Just use feature detection for each set of tests we might test, and if the runtime supports it (whether because it’s a new enough version or because --harmony is used) then we run those tests.

I also don’t like the idea of mixing a JSON file into the codebase. I know its function would be primarily to store data, hence it’s good to use a “data” file type, but editing that file would look very different from editing the rest of the codebase. Could we just use a regular .coffee file that simply exports an object?

@rdeforest
Copy link
Contributor Author

I think you just cleared up all my confusion and I think I can keep the PR small. I should have something in an hour or so (fingers crossed).

rdeforest added a commit to rdeforest/coffeescript that referenced this issue Mar 2, 2018
rdeforest added a commit to rdeforest/coffeescript that referenced this issue Mar 3, 2018
@rdeforest rdeforest changed the title Proposal: Modify Cakefile to generalize handling of tests requiring advanced V8 features Proposal: generalize exempting of tests requiring advanced V8 features Mar 4, 2018
GeoffreyBooth pushed a commit that referenced this issue Mar 13, 2018
@GeoffreyBooth
Copy link
Collaborator

Fixed via #5003.

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

2 participants