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

(#48) - inline some of the test calls #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

calvinmetcalf
Copy link
Contributor

inlines all the calls to testPromiseResolution, a start towards inlining more of the tests.

@juandopazo
Copy link

What if we use templates to generate the JS test file?

@calvinmetcalf
Copy link
Contributor Author

If the likelihood of a spec 1.2 is high then that might be a good idea, if
it's low then this is probably easier.

On Mon, Mar 17, 2014 at 10:58 AM, Juan Ignacio Dopazo <
[email protected]> wrote:

What if we use templates to generate the JS test file?

Reply to this email directly or view it on GitHubhttps://github.com//pull/56#issuecomment-37824988
.

-Calvin W. Metcalf

@domenic
Copy link
Member

domenic commented Mar 18, 2014

Yeah, I don't think this is the way to go---hurts maintenance burden too much. I thought the strategy of a smoke-test TDD-able suite discussed in #48 would be more useful coming out of that discussion.

@calvinmetcalf
Copy link
Contributor Author

ok switched it over to smoke testing, so far I added throwing after fulfilling with a promise, (the one that got my library) can anyone thing of some other ones to check?

@domenic
Copy link
Member

domenic commented Mar 26, 2014

This looks great. I will try to merge it soon, although perhaps modified as part of the overall smoke-testing plan.

The overall smoke-testing plan

A new file, or perhaps set of files, containing very easy-to-debug representative smoke tests for each point in the spec. E.g., maybe a single test that .then({}, function () { }) doesn't throw. Or a bunch of tests for the individual edge cases of [[Resolve]], written out in full---getters, double-fulfilling thenables, etc.

Unlike the raw tests, timeout errors on failure are not acceptable in the smoke tests; they need to be easy to debug if possible. (For example, testing f.then(function () { done(); }) for some promise f that should be fulfilled is not good enough; instead it should be f.then(function () { done(); }, function () { done(new Error("Should not be rejected!")); }).)

The test hierarchy should be flat, to ease porting to test-262 format later.

There will be a way to easily run only the smoke tests, but this should output a giant warning that passing these does not guarantee compliance.

I'll try to incorporate this, plus maybe a couple more, as our first example to get the ball rolling. Then maybe people can start contributing tests.

@centurianii
Copy link

I'm not a Node expert! Can we have the tests in simple javascript so we can test in browser?

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

Successfully merging this pull request may close these issues.

4 participants