stalniy / bdd-lazy-var

Provides UI for testing frameworks such as mocha, jasmine and jest which allows to define lazy variables and subjects.
MIT License
162 stars 14 forks source link

Using testing framework context in variable definitions #115

Closed vonagam closed 3 years ago

vonagam commented 3 years ago

Sometimes a variable definition might have a side effect that should be reverted in an after hook, but getting resulted value for cleanup through get or a dollar variable is not really correct, since it can be overshadowed by new definition in a sub suite... (Also the moment you call get the value will be evaluated and if it was not before then you just called a function just to undo its result...)

Or other way around: you might already have something on context object that you want to use in a definition.

Exposing context (as this value through apply, i assume) will solve such cases and allow for better communication between a testing framework and lazy vars.

Thoughts?

stalniy commented 3 years ago

I'm currently thinking on how to be more testing framework independent, improve typescript support and add support for jest-circus.

So, don't know whether it will be possible to do in the next version. I could propose to additionally add cleanup function and register definition like this:

def('var', {
  eval() {
  },
  cleanup() {
  }
})

But first, I'd like to see some particular examples, of how/why/when you use this.

stalniy commented 3 years ago

or maybe this:

def('var', () => {
  const value  = // do some stuff
  return { 
    value, 
    cleanup: () => /* cleanup value */
  }
})
vonagam commented 3 years ago

Yeah, i saw the possible new api which is more type friendly and like it. (I myself right now in process of rewriting things that i have into typescript, including some test helpers related to bdd-lazy-var.)

As i said this is about two-way street, so it can be about cleaning up or about getting access to exposed data on a context.

So, for example, here is my current hack/workaround for using async non-lazy definitions with lazy vars:

function defAsync(name, definition) {
  const ref = {};

  def(name, () => {
    if (!ref.hasOwnProperty('value'))
      throw new Error(`Cannot access async lazy "${name}" before corresponding evaluation hook has run.`);
    return ref.value;
  });

  beforeEach(`defAsync: "${name}" eval`, async () => {
    ref.value = await definition();
  });

  afterEach(`defAsync: "${name}" cleanup`, async () => {
    delete ref.value;
  });
};

My concern is that ref is globalish and such approach could lead to problems if used with parallel test runner (instead of mocha). I could have used context.currentTest instead of ref and a unique symbol instead of value key if i had access to a context.

Maybe i am overthinking this... Feel free to close. (I also found out that i already have access to a test context through ctx property on a suite context.)

stalniy commented 3 years ago

113 is the proposal to add analog of let! which can be used with async defs because it anyway will be evaluated on every beforeEach

vonagam commented 3 years ago

Cool. (I also have defOnce helper, which is exact same thing as defAsync, but uses before/after instead of beforeEach/afterEach. Mention just in case.)

If you need specific example with teardowns - i also have defSinon which expects a resulted value to be a single spy or collection of them. And in afterEach it does restore or resetHistory (if there is nothing to restore).

vonagam commented 3 years ago

If it comes to api design, i think third optional argument for cleanup is better (simple and backward compatible) than cleanup property:

function def<T = any>(name: string, implementation: () => T, cleanup?: (value: T) => void | Promise< void >): void;

Because cleanup can be genuine property on a lazy value.

(Also for record, i think there is a problem with defAsync implementation that i posted: because of cleanup function if you call lazy value first time in a hook after cleanup the value will be undefined... I initially did not have this cleanup hook, added when posting to avoid potential hanging values in a memory and for proper error throwing on access out of order.)

vonagam commented 3 years ago

I'll close the issue: it is possible to achieve what i want with what available and not sure about cost/benefit.