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

Enhanced control of test context #3485

Closed
4 tasks done
papercuptech opened this issue Sep 24, 2018 · 11 comments
Closed
4 tasks done

Enhanced control of test context #3485

papercuptech opened this issue Sep 24, 2018 · 11 comments

Comments

@papercuptech
Copy link

papercuptech commented Sep 24, 2018

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend avoiding the use of globally installed Mocha.

Description

When I install and use 'mocha-ctx', I get enhanced control of context! It seems like it keeps doing this every time I use it with mocha.

Steps to Reproduce

npm i mocha-ctx

require('mocha-ctx')
describe('mocha-ctx', () => {
  before(() => {
    context({
      cool: 'stuff!',
      fn: () => 'I see cool ' + context().cool
    })
  })
  it('can do this', function() {
    this.someProp = 10
    assert(this === context())
  })
  it('can now provide context (i.e. "this") to lambdas', () => {
    assert(context().someProp === 10)
  })
  describe('layer 1', () => {
    it('can do other stuff too', () => {
      // this will NOT create a 'cool' property on layer 1 context
      // but will actually set 'cool' property at top context (or wherever
      // 'cool' was defined in context hierarchy)
      context().cool = 'way'
      assert(context().fn() === 'I see cool way')
      context().timeout(-42)
      context().skip()
    })
  })
})

Expected behavior: [What you expect to happen]
That someone, somewhere will jump up and down in joy.

Actual behavior: [What actually happens]
That they will smile too!, and that mocha will want these features as part of mocha.

Reproduces how often: [What percentage of the time does it reproduce?]
100%

Versions

1.0.0-a.0

Additional Information

Additional 'bugs'

  • Backwards compatible (explicit opt in).
  • Access test context inside lambda functions without passing as parameter.
  • Every test gets its own context, sharing parent context, hiding private context.
  • Explicitly declared contextual properties and functions, so lower contexts can set higher shared properties.
  • Contextualized 'global' || 'window', with stronger access and use detection on top of leak detection.

More details at https://github.com/papercuptech/mocha-context

@plroebuck
Copy link
Contributor

Have you taken a look at PR #3399?

@papercuptech
Copy link
Author

papercuptech commented Sep 24, 2018

@plroebuck That PR appears to not be backwards compatible and seems to primarily address providing lambdas access to context.

mocha-ctx also enables lambdas to access context, but in a backwards compatible way (i.e. does not require changing what is passed to lambda, so passing 'done' still works). Further, this is a minor feature of mocha-ctx, but should address #2767, #2657, #2018, #1856.

mocha-ctx's larger value is being able to let lower contexts set higher context properties that are being shared, and that each test truly has its own context, which should address #2014, #2140, #2914, #2977, #797.

mocha-ctx also provides control of globals in context; i.e. things running in a given context can see globals differently, or be restricted from even accessing some, and mocha-ctx will correctly housekeep saving and restoring as hierarchical contexts are entered and left, which should address #2656.

Additionally, the behavior mocha-ctx provides is only explicitly enabled for a given context (and all things running in it), so you can have some sub-contexts that use it and some that don't, which should allow for easier adoption.

Lastly, all these features should make it much easier to compose contexts and tests, so more types of integration testing can more easily be done with mocha.

@papercuptech
Copy link
Author

papercuptech commented Sep 24, 2018

@plroebuck things like this are now a little easier.

var function testUtil() {
  assert(context().testProp === true)
}

var tests = () => {
    it('uses shared context', () => {
      assert(context().shared === 1)
    })
    it('uses test context', () => {
      context().testProp = true
      testUtil()
    })
    it('uses test context, but fails', () => {
      // testUtil will fail, as 'testProp' not defined;
      // each test now has own context
      testUtil()
    })
    it('uses global', () => {
      assert(someGlobal === 'read only')
    })
    it('sets shared context', function() {
      this.shared = 42
      // context().shared = 42 would work too
    })
    it('really is "this" context', () => {
      context().skip()
    })
}

describe('define context tests need', () => {
  before(() => {
    // here is where 'magic' happens, and also activates per-test context
    context({
      shared: 0,
      sharedFn: () => context().shared - 42
    })
  })
 
  describe('provide specific context', () => {
    before(() => {
      context().shared = 1
      
      context({
        globals: {
          someGlobal: undefined
          // now anything that gets or sets 'someGlobal' will throw
        }
      })
    })

    // fails merely from attempting to access (get) global 'someGlobal'
    tests()
  })

  // context can still be used as alias for 'describe', but not other way around
  context('provide some other specific context', () => {
    before(() => {
      context().shared = 1000
      // can get but not set 'someGlobal'
      context({globals: {someGlobal: 'read only'}})
    })

    // fails on context().shared not being 1 (it's 1000)
    tests()

    it('shared', function() {
      assert(this.shared === 42)
      assert(this.sharedFn() === 0)
    })
  })

})

@Munter
Copy link
Contributor

Munter commented Sep 24, 2018

What is the value of mocha-ctx over just using scope defined variables?

describe('mocha with context variables', () => {
  const context = {
    cool: 'stuff!',
    fn: () => 'I see cool ' + context.cool
  };

  it('can not do this: what would the point of that even be anyway?', function() {
    assert(this === context)
  })

  describe('layer 1', () => {
    it('can do other stuff too', () => {
      // this will NOT create a 'cool' property on layer 1 context
      // but will actually set 'cool' property at top context (or wherever
      // 'cool' was defined in context hierarchy)
      context.cool = 'way'
      assert(context.fn() === 'I see cool way')
      this.timeout(-42)
      this.skip()
    })
  })
})

@papercuptech
Copy link
Author

papercuptech commented Sep 24, 2018

@Munter In your example, the test 'can do other stuff too' can not be composed into other describes. Your example test is the same functionally, but is coupled and hard coded to the const context. You can do what you've shown, but mocha-ctx offers much more compos-ability. Look at my comment and sample code just preceding your comment.

Your test can not do this: what... is missing the point. mocha-ctx lets () => {} get tothis via context() without passing as an argument and thus not breaking compatibility; in fact anything can get to context().

@Bamieh
Copy link
Contributor

Bamieh commented Sep 24, 2018

Thanks for the serious effort writing this.

Sadly I feel that this feature adds extra complexity to Mocha.

  • Your library mocha-ctx is already doing this.
  • It is not a core feature in unit testing. On the contrary sharing some state between root suites is considered a bad practice in unit testing.
  • I don't see the community asking for this feature, all issues can be solved with simple lexical scoping if necessary.

@papercuptech
Copy link
Author

@Bamieh

@papercuptech
Copy link
Author

papercuptech commented Sep 24, 2018

@Bamieh Consider these simplifications:

  • Calling context() becomes the normative way of always getting access to current test context.

    • When called from function(s) running as hook or test callback, and any funcs those cb's call
    • Can still use it('test', function() {this}), but can now always use it('test', () => {context()}) or it('test', function() {context()})
  • Every test now gets its own completely isolated context that has nothing shared by default

    • If something needs to be shared as context, ideally only with other 'sub-context' providers, but if necessary with tests too, it must be explicitly declared to be as such (and can actually be locked as 'read only'), and with respect to properties, mocha-ctx defines getter/setters, so setting a context property actually sets the value in the context which defines the property, which is the intuitive thing.
  • Globals are usually not good, but sometimes have to be dealt with, and now each test context can get its own 'global' environment (mostly)

    • Can see (fail) if things are trying to read/write known globals, contextually
    • Can mock global just for a particular context test(s) are running in. This can help create tests meant to be used to first identify, then move away from using globals.
  • Provides a means to completely decouple defining context, from implementing an instance of context, to something running in a type of context. This should make fluid composition of context and tests nicer, and mocha could be used for more kinds of testing beyond just unit testing, like small to medium integration testing (not that it can't already be).

  • Its all backwards compatible, btw.

  • mocha-ctx is currently brittle, with respect to how it patches mocha; ideally it should just be part of it.

If you truly believe these things are not of the same ethos as mocha, and testing generally, please feel free to close this issue, although it would be interesting to hear from others on the matter.

@aleung
Copy link

aleung commented Sep 25, 2018

Calling context() rather then this would be helpful. Then I can always use arrow function. Now I have to temporary disable the tslint rule only-arrow-functions in order to access this in test case.

@papercuptech
Copy link
Author

@Bamieh

context() becomes the singular way to access test context.

  • This lets us use lambdas all the time, without breaking the existing API.

Default becomes each test truly has it's own context()

  • Today one test can this.foo = 1 and the next test will see this.foo; i.e. mocha currently shares context between tests by default, contrary to your statement "sharing state between contexts [and tests] is a bad practice".

Shared context happens

  • As you stated, "all issues can be solved with simple lexical scoping", confirming shared context is an issue, (not to mention relying on lexical scoping creates greater technical coupling between the things controlling context and the things running in that context).
  • Calling context({ someProp: 'shared' }) from a before() hook becomes the singular and only way to explicitly declare shared context, because the default becomes that nothing is shared. (Note: calling context({}) activates per-test context only within that context (and descendants), making the context-per-test semantic opt-in, explicit, and backwards compatible)
  • An explicitly shared property now lives at the level of the context, so if a lower context changes, it actually changes at and within the higher context which is the intuitive behavior, unlike today where, because of how js's prototyping works, the un-intuitive thing happens.
  • In those horrible, horrible cases when state is shared, now its made explicit and put into control of the host test environment (because a function, context({prop: 'state'}), must be called).

Globals are pretty bad, except when they're not

  • mocha's default is to expose itself through globals
  • mocha can only detect when a new global is created, but with these changes, it would also be able to detect a test merely attempting to get a global, and set an existing global.
  • In those horrible, horrible cases when things using globals must be tested, we now have a little more control over them in a littler easier way (i.e. managed globals are automatically saved and restored as contexts are entered and left, and it's dead simple to set a global to a mock for a given context).

A Controlled Environment is essential to testing

  • All the above are meant to bring greater control and isolation of the context a test must run in, doing so in a very consistent, intuitive, and simplifying way.
  • All the above are intended to make it easier to technically decouple the things defining and managing context, from the things running in a context when such is required, unlike lexical scoping. Among other things, this should facilitate easier composition of contexts and tests.

@Bamieh Would you please elaborate on why you believe mocha should not be providing this kind of control over the environment; I had previously believed controlling the environment in which things are tested within was exactly what mocha was meant to facilitate.

@papercuptech
Copy link
Author

Out of scope

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

5 participants