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

Possible async problem #21

Open
jamestalmage opened this issue Nov 26, 2015 · 3 comments
Open

Possible async problem #21

jamestalmage opened this issue Nov 26, 2015 · 3 comments

Comments

@jamestalmage
Copy link
Contributor

Now that we support async functions, is the fact that this is in a closure going to be a problem?

I am near certain the answer is yes!

Look at this code:

test('a', t => {
  t.is(await promiseA1(), await promiseA2());
});

test('b', t => {
  t.is(await promiseB1(), await promiseB2());
});

Now, assume both following sequence of events:

  1. test a starts
  2. test a halts as it awaits the resolution of promiseA1()
  3. test b starts
  4. test b halts as it awaits the resolution of promiseB1()
  5. promiseA1() resolves, and test a resumes.
  6. test a halts as it awaits the resolution of promiseA2()
  7. promiseB1() resolves, and test b resumes.
  8. test b halts as it awaits the resolution of promiseB2()
  9. promiseB2() resolves, and test b resumes.
  10. test b ends
  11. promiseA2() resolves, and test a resumes.

It seems like events is going to be corrupted by step 12.

I think AVA is going to need to create a new enhanced assert object for each run.

@twada - thoughts?

@twada
Copy link
Member

twada commented Dec 4, 2015

@jamestalmage Ah, Good catch! Thank you for sharing your thought!

import assert from 'power-assert';

const tests = [];

function test (name, body) {
    tests.push({
        name,
        body
    });
}

function later (millis, value) {
    return new Promise((resolve, reject) => {
        setTimeout(() => {
            console.log(`captured [${value}]`);
            resolve(value);
        }, millis);
    });
};

test('testA', async () => {
    const val1 = 'a1';
    const val2 = 'a2';
    const delay1 = 1000;
    const delay2 = 6000;
    try {
        assert.equal(await later(delay1, val1), await later(delay2, val2));
    } catch (e) {
        console.log(e.message);
    } finally {
        console.log(`done [testA]`);
    }
});

test('testB', async () => {
    const val3 = 'b1';
    const val4 = 'b2';
    const delay3 = 2000;
    const delay4 = 3000;
    try {
        assert.equal(await later(delay3, val3), await later(delay4, val4));
    } catch (e) {
        console.log(e.message);
    } finally {
        console.log(`done [testB]`);
    }
});

for (let {name, body} of tests) {
    console.log(`running [${name}]`);
    body();
}

Produces power-assert output that is totally broken.

$ babel-node interleaving_promises.js
running [testA]
running [testB]
captured [a1]
captured [b1]
captured [b2]
  # interleaving_promises.js:41

  assert.equal(await later(delay3, val3), await later(delay4, val4))
               |                          |           |       |
               |                          |           |       |a2"
               |                          |           |000    "b2"
               "b1"                       "b2"        3000

done [testB]
captured [a2]
  # interleaving_promises.js:27

  assert.equal(await later(delay1, val1), await later(delay2, val2))
               |           |       |      |
               "a1"        2000    "b1"   "a2"

done [testA]
$ 

I'll fix this problem in this issue

@jamestalmage
Copy link
Contributor Author

Thanks @twada, I meant to write that test and verify this.

Could we maybe introduce a runtime helper? I think Babel 6 now includes an easy way to do that (from what I have browsed of the source.

t.is(await foo(), await bar());
var __powerAssertRuntime = require(...); // once at the top of the file.

 // create a unique name - I think Babel might have a tool for this.
const capt_1 = new __powerAssertRuntime.Capture(); 
const capt_2 = new __powerAssertRuntime.Capture(); 

t.is(await capt_1._expr(capt_1._capt(foo(), {powerAssertContext:...})), await capt_2._... )

@twada
Copy link
Member

twada commented Dec 4, 2015

@jamestalmage Yeah, I'm thinking about introducing runtime helper too.

This is my thought but I'll make prototypes of both.

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

No branches or pull requests

2 participants