trailsjs / generator-trails-old

:package: Trails Generator.
Other
6 stars 8 forks source link

Mocha tests should not use arrow functions #69

Closed disciple-dev closed 7 years ago

disciple-dev commented 7 years ago

From Mocha Docs:

Passing arrow functions (“lambdas”) to Mocha is discouraged. Due to the lexical binding of this, such functions are unable to access the Mocha context. For example, the following code will fail due to the nature of lambdas:

describe('my suite', () => {
  it('my test', () => {
    // should set the timeout of this test to 1000 ms; instead will fail
    this.timeout(1000);
    assert.ok(true);
  });
});

I'm thinking particularly of the templates for things like services, policies, etc.

konstantinzolotarev commented 7 years ago

@metzuda it really depends on need. In most cases there is no real need to use mocha context at all. And creating pure functions not really needed. But with arrow functions tests becomes a bit cleaner.

@trailsjs/maintainers Any opinion on this ?

disciple-dev commented 7 years ago

@konstantinzolotarev granted, the whole reason I noticed this was because I did need mocha context for tests, and noticed that they were all set up as arrow functions, which seems to be a purely style issue, here.

tjwebb commented 7 years ago

Crap, I use arrow functions everywhere in mocha. everywhere. Is there a larger discussion to be had about whether we need to alter this style practice?

disciple-dev commented 7 years ago

I'm not really sure. Within most Trails' modules it doesn't look like it's an issue, and I don't think it's something that most people will run into — on the other hand, I feel like encouraging what may end up being a bad habit just because we like arrow functions isn't the way to go.

If someone has to go through and re-write a bunch of generated tests then the generator has actually cost them time. Better to be safe out of the box, I say.

ryanwilliamquinn commented 7 years ago

I think it is only an issue if you ever need to use this, which is rare, and can be handled as a one-off.

jaumard commented 7 years ago

Closing this, in the generator itself we don't care. If it's a problem for generate class, issue should be create on Trails repo (it contains sources that are copy by this generator)