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

--delay does not delay execution of suite functions as expected. #2257

Closed
brucejo75 opened this issue May 15, 2016 · 9 comments
Closed

--delay does not delay execution of suite functions as expected. #2257

brucejo75 opened this issue May 15, 2016 · 9 comments

Comments

@brucejo75
Copy link

I am trying to see if I can find a solution to chimp #355.

I tried the demo provided by @boneskull for using --delay from #1628.

This demo did not demonstrate the expected benefit to using --delay. Maybe I am expecting something it does not do?

Expected

--delay flag would delay execution of suite functions or child suite functions until run() is called.

Observed

suite functions are executed prior to run() call being made.

I added some console output and I think that this shows that the --delay flag does not allow you to initialize variables prior to any suite functions code being run. You can see from my test console the sequence is:

  • run describe1
  • run describe2
  • handle nextTick callback

Result: foo is only properly set within the it block.

This is exactly the same as not using the --delay flag.

Modified test @boneskull test

var foo;

// process.nextTick to be replaced with your async setup function
process.nextTick(function() {
  foo = 'bar'; // or "credentials" object w/ user id
  console.log("in nextTick");
  run(); // this is exposed when running mocha with the --delay flag
});

describe(require('path').basename(__filename), function() {
  console.log('describe1 foo: ' + foo);
  describe('describe: '+ foo + ' ===? bar', function() {
    console.log("describe2 foo: " + foo);
    it('it: '+ foo + ' ===? bar', function() {
      console.log('within it: ' + foo + ' ===? bar')
      require('assert')(true);
    });
  })
});

Modified Console output

mocha --delay
describe1 foo: undefined
describe2 foo: undefined
in nextTick


  test.js
    describe: undefined ===? bar
within it: bar ===? bar
      √ it: undefined ===? bar


  1 passing (0ms)
@ScottFreeCode
Copy link
Contributor

ScottFreeCode commented May 15, 2016

(EDITTED TO CLARIFY) I believe the delay option only delays actually running the tests. If you want to delay the describe calls themselves until whatever asynchronous stuff happens that's needed for the describe calls themselves, you'll want the describe calls not to be made in the first place till the asynchronous execution (the same place you call run, but still before run), e.g.:

// process.nextTick to be replaced with your async setup function
process.nextTick(function() {
  console.log("in nextTick");
  defineTests('bar'); // or "credentials" object w/ user id
  run(); // this is exposed when running mocha with the --delay flag
});

function defineTests(foo) {
  describe(require('path').basename(__filename), function() {
    console.log('describe1 foo: ' + foo);
    describe('describe: '+ foo + ' ===? bar', function() {
      console.log("describe2 foo: " + foo);
      it('it: '+ foo + ' ===? bar', function() {
        console.log('within it: ' + foo + ' ===? bar')
        require('assert')(true);
      });
    });
  });
}

@ScottFreeCode
Copy link
Contributor

Tangentially, from the linked issue it sounds like you may not actually need to delay the describe calls, just the tests anyway, something more along these lines:

describe("asynchronously dynamically generated tests", function() {
  doSomethingToGetDataAsynchronously(function(asynchronouslyRetrievedData) {
    asynchronouslyRetrievedData.forEach(function(entry) {
      it("test for " + entry, function() {
        assertStuffAbout(entry);
      });
    });
    run();
  });
});

@brucejo75
Copy link
Author

brucejo75 commented May 16, 2016

Thanks @ScottFreeCode. I will have to take a look at that pattern and see if I can jam it to get what I need done.

Unfortunately, the global variable I want to access in Chimp (server) is only available in the it scope. Today server is declared inside a before scope. see https://github.com/xolvio/chimp/blob/master/src/lib/mocha/mocha-helper.js#L9

Referring to your example above my doSomethingToGetDataAsynchronously (server) is undefined in a describe scope.

I wanted to investigate the --delay flag as an option that Chimp could take advantage of in the declaration of the global variable server, hoping to get it defined prior to a describe call. So that is off the table if --delay does not delay suite declarations.

I think I could achieve what I want if #1628 produced a beforeDescribe hook. Chimp could declare the server variable in a beforeDescribe hook.

Any other thoughts are more than welcome!

@ScottFreeCode
Copy link
Contributor

Hmm... Yeah, I'm not sure even an option to delay describe would help with that; if I recall correct hooks such as before don't run until you ask Mocha to run the tests.

I've looked a little more closely at the linked issue, and one thing's not clear to me -- what is happening between the top-level code running and the before hook running that makes it possible to set up the server variable in the before hook but not in the top-level code? Without a clear idea of that, I'm not sure even having a hypothetical hook that runs before describe would necessarily resolve the issue -- if Mocha ran it immediately when the describe calls were made, it would still be happening effectively at the same point as top-level code and run into the same problem setting up server as when trying to do so from top-level code, whereas for Mocha to wait to run it until mocha.run() would mean (as far as I can tell) not just an addition of code for that hook but a major overarching design change to be able to save and delay the describe functions' callbacks till then and yet keep everything functioning otherwise the same as it does now.

I'm not sure I'm reading this right, but it rather looks like this line is when the script containing the server setup is executed (although the server setup itself is currently embedded in a Mocha hook and won't run until mocha.run() later): https://github.com/xolvio/chimp/blob/master/dist/lib/mocha/mocha-wrapper.js#L33 and this line is where mocha.run() happens and should trigger the before hook where the server setup is actually done: https://github.com/xolvio/chimp/blob/master/dist/lib/mocha/mocha-wrapper.js#L46 In between all I see is adding the test scripts to Mocha; as far as I can tell there shouldn't be any difference between the code as it is now and just removing before(function () { and }); (except, of course, that server would then be available while defining the tests rather than only when running them). Yet according to the linked issue, running it in the one place works (albeit too late) and running it in the other gets an error for lack of a fiber -- how is that fiber being started? Could you walk through it in the debugger and try to find where the fiber is being started that makes the difference? (I tried to run a minimal test case with debugger myself and for some reason couldn't get it to work following the instructions in the manual...)

@brucejo75
Copy link
Author

brucejo75 commented May 16, 2016

Chimp wraps all the mocha hooks in fibers in this mocha-fiberized-ui.js

I tried pulling the code out of the before hook and running from a fiberized function but this got lost on a wait. (which I did not investigate very deeply)

the idea of a beforeDescribe hook would be that Chimp could fiberize the hook and then make the initialization calls in there, so it would align with the existing pattern. But if as you say, not much happens between so we should be able to declare server without issue.

I will take a deeper look when I spring some time loose. Unfortunately for me, once I start gnawing on something I have a hard time stopping.

@ScottFreeCode
Copy link
Contributor

Ah, now I see...

My inclination is to say that, if the fiberized stuff being set up in the before hook is meant to be the same variable across all those fibers, it probably shouldn't require a fiber to be created in the first place; or, if it should actually be unique per fiber instead (which isn't the current behavior if I understand correctly, so it's probably not the case...), then it should be set up in the process of fiberizing the tests so that there's one per test. But I guess I'd have to figure out why it doesn't work to put it in its own fiber instead of one of the fiberized hooks.

(I also wonder if the fiberization could be done in such a way that any Mocha interface can be chosen... but that's something to dig into some other time, I guess.)

Actually, though, is the server and other globals actually what need to be set up in a fiber currently, or is it something else? Does it work if you move chimpHelper.init(); out of the before hook but otherwise leave the other stuff in the hook?

I totally hear you about having to get to the bottom of things!

@boneskull
Copy link
Contributor

I'm unclear if this is a bug, feature request, or "user error"..?

@ScottFreeCode
Copy link
Contributor

If I've understood correctly, it seems to be an issue with Chimp:

  1. Needs a fiberized function to set up certain objects.
  2. Fiberizes Mocha's hooks.
  3. Sets up these objects in a root-suite before hook.
  4. Some users of Chimp want to use these objects to dynamically define tests using --delay/run().
  5. But they can't since the objects aren't set up till a before hook runs.

Hypothetically, Mocha and Chimp could work together to resolve that by creating a new Mocha hook that runs even before describes are run and is fiberized by Chimp too, assuming there aren't complications in that solution anyway. However, it's not clear to me whether/why Chimp couldn't just set up a fiber to set up the objects without needing Mocha's hooks, and it looks like the particular server object in question might be able to be set up outside of the fiberized functions even though some of the other objects can't be.

@brucejo75
Copy link
Author

Great summary @ScottFreeCode. I have investigated some of

However, it's not clear to me whether/why Chimp couldn't just set up a fiber to set up the objects without needing Mocha's hooks, and it looks like the particular server object in question might be able to be set up outside of the fiberized functions even though some of the other objects can't be.

I need to investigate further but it is way back burner right now. So I will close the issue pending more info.

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

3 participants