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

Stuck at Idempotent Routing -> new ES2015 BDD style test cases failing while old Tinytest cases works (now on right branch) #456

Open
wants to merge 7 commits into
base: ssr
Choose a base branch
from

Conversation

tjmonsi
Copy link

@tjmonsi tjmonsi commented Dec 18, 2015

I understand what idempotent routing does (which should not re-run the action function when Router.go is called). I just don't get it why it fails to work on the test case when I transferred it to new ES2015 BDD style test case.

Here's the snippet of the test case on the old code.

Tinytest.addAsync('Client - Router - idempotent routing - action',
function(test, done) {
  var rand = Random.id();
  var pathDef = '/' + rand;
  var rendered = 0;

  FlowRouter.route(pathDef, {
    action: function(params) {
      rendered++;
    }
  });

  FlowRouter.go(pathDef);

  Meteor.defer(function() {
    FlowRouter.go(pathDef);

    Meteor.defer(function() {
      test.equal(rendered, 1);
      done();
    });
  });
});

Tinytest.addAsync('Client - Router - idempotent routing - triggers',
function(test, next) {
  var rand = Random.id();
  var pathDef = '/' + rand;
  var runnedTriggers = 0;
  var done = false;

  var triggerFns = [function(params) {
    if (done) return;

    runnedTriggers++;
  }];

  FlowRouter.triggers.enter(triggerFns);

  FlowRouter.route(pathDef, {
    triggersEnter: triggerFns,
    triggersExit: triggerFns
  });

  FlowRouter.go(pathDef);

  FlowRouter.triggers.exit(triggerFns);

  Meteor.defer(function() {
    FlowRouter.go(pathDef);

    Meteor.defer(function() {
      test.equal(runnedTriggers, 2);
      done = true;
      next();
    });
  });
});

and here's the new BDD style

context('Idempotent Routing', () => {
  it('should call action', done => {
    const rand = Random.id();       // var rand = Random.id();
    const pathDef = `/${rand}`;     // var pathDef = '/' + rand;
    let rendered = 0;               // var rendered = 0;

    router.route(pathDef, {         // FlowRouter.route(pathDef, {
      action() {                    //   action: function(params) {
        rendered++;                 //     rendered++;
      }                             //   }
    });                             // });

    router.go(pathDef);             // FlowRouter.go(pathDef);

    Meteor.defer(() => {            // Meteor.defer(function() {
      catchable(done, () => {
        router.go(pathDef);         //   FlowRouter.go(pathDef);

        Meteor.defer(() => {        //   Meteor.defer(function() {
          catchable(done, () => {
            if (Meteor.isClient) expect(rendered).to.be.equal(1);
                                    //     test.equal(rendered, 1);
            done();                 //     done();
          });                       //   });
        });                         // });
      });
    });
  });

  it('should call triggers', done => {
    const rand = Random.id();
    const pathDef = `/${rand}`;
    let runnedTriggers = 0;
    let finish = false;

    const triggerFns = [() => {
      if (finish) return;
      runnedTriggers++;
    }];

    router.triggers.enter(triggerFns);

    router.route(pathDef, {
      triggersEnter: triggerFns,
      triggersExit: triggerFns
    });

    router.go(pathDef);

    router.triggers.exit(triggerFns);

    Meteor.defer(() => {
      catchable(done, () => {
        router.go(pathDef);

        Meteor.defer(() => {
          catchable(done, () => {
            if (Meteor.isClient) expect(runnedTriggers).to.be.equal(2);
            finish = true;
            done();
          });
        });
      });
    });
  });
});

The output turns out this way:

idempotent routing - action - OK
idempotent routing - triggers - OK
...
Idempotent Routing - should call action
fail — assert_equal - message expected 2 to equal 1 - expected 1 - actual 2 - not
Idempotent Routing - should work on triggers
fail — assert_equal - message expected 6 to equal 2 - expected 2 - actual 6 - not 

Can you check what I did wrong?


api.addFiles('test/client/_helpers.js', 'client');
api.addFiles('test/server/_helpers.js', 'server');

api.addFiles('test/client/loader.spec.js', 'client');
api.addFiles('lib/__tests__/loader.js', ['client', 'server']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't put this on the server. Try to put them in client/__tests__. Also run them only in the client. There a lot of stuff you don't wanna run in the server.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually lib/__tests__/loader.js is just test/common/loader.spec.js
The test/client/loader.spec.js is in client/__tests__/loader.js

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay. Right now, I'm in the middle of something. Will do a review tonight. Hope you won't mind that :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's alright. I'm doing something else right now as well. :)

On Fri, 18 Dec 2015 15:46 Arunoda Susiripala notifications@github.com
wrote:

In package.js
#456 (comment):

api.addFiles('test/client/_helpers.js', 'client');
api.addFiles('test/server/_helpers.js', 'server');

  • api.addFiles('test/client/loader.spec.js', 'client');
  • api.addFiles('lib/tests/loader.js', ['client', 'server']);

okay. Right now, I'm in the middle of something. Will do a review tonight.
Hope you won't mind that :)


Reply to this email directly or view it on GitHub
https://github.com/kadirahq/flow-router/pull/456/files#r48001389.

@tjmonsi
Copy link
Author

tjmonsi commented Jan 15, 2016

I am going to go back and add more tests here. :)

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.

2 participants