bdd-lazy-var icon indicating copy to clipboard operation
bdd-lazy-var copied to clipboard

bdd-lazy-var and jest-circus integration

Open pmoons opened this issue 3 years ago • 40 comments

On version 2.5.4 of bdd-lazy-var, I am receiving the error:

Cannot read property 'getEnv' of undefined

    > 1 | import "bdd-lazy-var/global";
        | ^

      at Object.createUi (node_modules/bdd-lazy-var/global.js:648:44)
      at node_modules/bdd-lazy-var/global.js:782:37
      at node_modules/bdd-lazy-var/global.js:3:83
      at Object.<anonymous> (node_modules/bdd-lazy-var/global.js:6:2)
      at Object.<anonymous> (src/setupTests.js:1:1)

This is in src/setupTests file, which (it being a create-react-app app), is the file specified as the jest argument in setupFilesAfterEnv.

create-react-app v4.0.0


My guess is it has to do something with this line of code, as it seems the interface/jest file just points to the interface/jasmine file.

Any ideas what I'm doing wrong here?

pmoons avatar Oct 27 '20 03:10 pmoons

Jest is a rewritten version of jasmine :) that’s why they have completely behavior.

The actual error is not clear for me. Is there any message?

stalniy avatar Oct 27 '20 05:10 stalniy

Ah I see. For a long time jest defined global jasmine variable maybe it was removed finally))

stalniy avatar Oct 27 '20 05:10 stalniy

what is your version of jest?

stalniy avatar Oct 27 '20 06:10 stalniy

just checked on [email protected] everything works fine. all tests in the repo pass. Also released a patch version with updates to the dependencies

stalniy avatar Oct 27 '20 07:10 stalniy

I believe my version of create-react-app is using 26.6.0

https://github.com/facebook/create-react-app/blob/master/package.json#L33

pmoons avatar Oct 27 '20 16:10 pmoons

you can actually clone this repo, try to update jest and run npm run test.jest-ui if it fails then I'll investigate. Will be able to look at it tomorrow only

stalniy avatar Oct 27 '20 16:10 stalniy

So, cloning this repo, updating jest to 26.6.0, running npm install and then npm run test.jest-ui results in this error:

⌂116% [peter:~/Workspaces/bdd-lazy-var] master(+297/-271) 48d0h57m34s ± npm run test.jest-ui

> [email protected] test.jest-ui /Users/peter/Workspaces/bdd-lazy-var
> SRC_FILE=index.js jest --findRelatedTests spec/interface_spec.js

 FAIL  spec/interface_spec.js
  ● Test suite failed to run

    Cannot find module '../index.js' from 'tools/jest.setup.js'

      2 |
      3 | require('../spec/config');
    > 4 | require(`../${uiFile}`);
        | ^
      5 | require('../spec/interface_examples');
      6 | require('../spec/default_suite_tracking_examples');
      7 | require('../spec/shared_behavior_spec');

      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:306:11)
      at Object.<anonymous> (tools/jest.setup.js:4:1)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        1.044 s
Ran all test suites related to files matching /spec\/interface_spec.js/i.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test.jest-ui: `SRC_FILE=index.js jest --findRelatedTests spec/interface_spec.js`
npm ERR! Exit status 1

Not the same one I was experiencing with create-react-app. Am I missing something?

pmoons avatar Oct 27 '20 17:10 pmoons

I guess, you also need to use npm run build, tests are run on bundled files

stalniy avatar Oct 27 '20 17:10 stalniy

Hmm, updating jest to 26.6.0 and running tests on this repo work fine, so it must be a problem on my end. I'll close this issue, and post here if I figure out what the problem is.

pmoons avatar Oct 28 '20 02:10 pmoons

Here is a bare-bones repo of the problem at hand: https://github.com/pmoons/test-bdd-lazy-var

Performing yarn test in the repo produces the same error I mentioned here.

All I did was:

  • npx create-react-app test-bdd-lazy-var
  • cd test-bdd-lazy-var
  • yarn add -D bdd-lazy-var
  • Add import 'bdd-lazy-var/global to src/setupTests

Not sure what is going on, but any ideas would be appreciated.

pmoons avatar Oct 28 '20 03:10 pmoons

Ok. Thanks. I’ll check it

stalniy avatar Oct 28 '20 03:10 stalniy

I think the issue is that you are trying to import it too early. At that stage global.jasmine doesn't exist. I think CRA puts src/setupTests into https://jestjs.io/docs/en/configuration#setupfiles-array but bdd-lazy-var needs to be executed in https://jestjs.io/docs/en/configuration#setupfilesafterenv-array

stalniy avatar Oct 28 '20 10:10 stalniy

just try to do console.log(global.beforeAll) if it's undefined, than my assumption is correct

stalniy avatar Oct 28 '20 10:10 stalniy

Anyway, ideally I need to add a check with the proper error message, so users are no confused. That's why I reopen this issue

stalniy avatar Oct 28 '20 10:10 stalniy

I think CRA does put it in setupFilesAfterEnv array, judging from this example: https://create-react-app.dev/docs/running-tests/#srcsetuptestsjs

Just weird that downgrading CRA to its previous release, 3.4.0, doesn't cause the issue.

pmoons avatar Oct 28 '20 16:10 pmoons

I think CRA does put it in setupFilesAfterEnv array, judging from this example: https://create-react-app.dev/docs/running-tests/#srcsetuptestsjs

Just weird that downgrading CRA to its previous release, 3.4.0, doesn't cause the issue.

I'm having the same problem, and the only thing that worked for me was downgrading the CRA as well. @stalniy Any ideas how to fix it with the new CRA version?

halilb avatar Dec 12 '20 19:12 halilb

Having the same issue. It was caused when updating react-scripts from 3 to 4, global.jasmine is undefined in react-scripts 4.

pete-mann avatar Jan 28 '21 10:01 pete-mann

I’ll check today whether access to rootSuite really necessary

stalniy avatar Feb 01 '21 05:02 stalniy

I finally understood why it throws. The issue is in new jest-circus package. This is a test runner that is going to replace jest starting from v27. And it looks like new CRA uses jest-circus in jest.config. This is a completely new test runner which does not depend on jasmine. That's why it throws.

Looks like this is a completely new feature for bdd-lazy-var!

stalniy avatar Feb 04 '21 08:02 stalniy

Screenshot from 2021-02-04 10-45-00

This is a screenshot of package.json after react-scripts eject command in the project described in this comment

stalniy avatar Feb 04 '21 08:02 stalniy

There is very little information about jest-circus and its internals, so it will take some time to create/adapt integration for it

stalniy avatar Feb 04 '21 08:02 stalniy

So, the workaround is too switch back jest-jasmine2 runner, however I don't know whether it's possible to change jest config without ejection.

Update: to do this - npm install jest-jasmine2 and specify testRunner in jest.config:

{
  testRunner: 'jest-jasmine2'
}

stalniy avatar Feb 04 '21 08:02 stalniy

I have some thoughts on how to improves typescript integration and this may change how variables are created and integrated with testing framework. Something like this

import { lazy } from 'bdd-lazy-var'

describe('suite', () => {
  const $ = lazy()
     .variable('num', 5)
     .variable('user', () => new User())

  it('tests', () => {
     expect($.user.age).toEqual($.num);
  })
})

Update: by using = operation I can infer types from variable functions for $ variable

stalniy avatar Feb 04 '21 09:02 stalniy

However the issue with this approach is that for every nested describe we need a separate variable but we can't use the same name every time. So, the only way to solve this in typesafe way is to use 2 variable names, one common like $ and another one common $parent:

import { lazy } from 'bdd-lazy-var'

describe('suite', () => {
  const $ = lazy()
     .variable('num', 5)
     .variable('user', () => new User({ age: $.num }));
  const $parentVars = $;

  it('tests', () => {
     expect($.user.age).toEqual($.num);
  })
 
  describe('nested suite', () => {
    const $ = lazy.extend($parentVars)
       .variable('num', 10);

    it('tests', () => {
       expect($.user.age).toEqual($.num);
    })
  })
})

Also it requires to disable eslint no-shadow rule to allow use the same variable name in nested desribes.

I can make the API a bit simpler by automatically tracking child/parent relationship but to keep variables safe, we still need to define a separate type:

describe('suite', () => {
  const $ = lazy()
     .variable('num', 5)
     .variable('user', () => new User({ age: $.num }));
  type ParentVars = typeof $; // <----- we need a type reference

  it('tests', () => {
     expect($.user.age).toEqual($.num);
  })
 
  describe('nested suite', () => {
    const $ = lazy<ParentVars>() // automatically inherits parent variables
       .variable('num', 10);

    it('tests', () => {
       expect($.user.age).toEqual($.num);
    })
  })
})

The 2nd approach complicates implementation and doesn't actually bring simplicity in comparison to the 1st one (only for js users).

stalniy avatar Feb 04 '21 10:02 stalniy

Thanks for nailing down what was causing this. I'm sure a lot of people will appreciate that workaround, and hopefully jest-circus compatibility in the near term.

As for improving typescript support, I like first the interface of the first example lazy.extends over the second, but not a fan of having to disable eslint no-shadow rule in either case.

pmoons avatar Feb 05 '21 03:02 pmoons

Would it be better to have a convention like $, $1, $2, ... where number after $ means a nesting level. Usually it shouldn’t go deeper $2.

import { lazy } from 'bdd-lazy-var'

describe('suite', () => {
  const $ = lazy()
     .variable('num', 5)
     .variable('user', () => new User({ age: $.num }));

  it('tests', () => {
     expect($.user.age).toEqual($.num);
  })
 
  describe('nested suite', () => {
    const $1 = lazy.extend($)
       .variable('num', 10);

    it('tests', () => {
       expect($1.user.age).toEqual($1.num);
    })
  })
})

In this case, no need to create $parentVars variable and no need to disable or add exceptions to no-shadow eslint rule. However, this still may be an option for some people.

As an alternative, for js users and those who are ok with weak typing in tests, we can do this (what is almost identical to the current behavior):

import { lazy } from 'bdd-lazy-var'

let $: Record<string, any>;
describe('suite', () => {
  $ = lazy()
     .variable('num', 5)
     .variable('user', () => new User({ age: $.num }));

  it('tests', () => {
     expect($.user.age).toEqual($.num);
  })
 
  describe('nested suite', () => {
    $ = lazy.extend($)
       .variable('num', 10);

    it('tests', () => {
       expect($.user.age).toEqual($.num);
    })
  })
})

I also will leave lazy().def(...) just for easier migration.

update: so those who want static types will need to follow suggested convention or use own one and who doesn’t care much can use $ variable everywhere.

stalniy avatar Feb 05 '21 21:02 stalniy

Just ran into this issue as well, then I realized who the OP was and wanted to say hi - what's up @pmoons!

ahmadabdul3 avatar Apr 11 '21 21:04 ahmadabdul3

I'm not sure ejecting or redefining variables in every describe block is going to work for everyone. I have 100s of tests that rely on bdd-lazy-var. Any updates on this?

Aror avatar Apr 27 '21 17:04 Aror

No updates. What interface do you use, bad-lazy-var/global?

stalniy avatar Apr 27 '21 17:04 stalniy

Yeah, we use def from bdd-lazy-var/global @stalniy

Do you have any plans in the immediate future to provide a fix for this or is that uncertain right now?

Aror avatar Apr 27 '21 18:04 Aror

@stalniy Just ran into this issue and we use bdd-lazy-var/getter and update the jest version to 27.0.5 in our codebase, most of our test cases are failed, would like to know do you have any plans within two months to provide a fix for this issue? image

GaoYu620 avatar Jun 29 '21 02:06 GaoYu620

Probably no, but I think that I will fix it till the end of this year. Just not a priority for me right now, sorry guys

stalniy avatar Jun 29 '21 03:06 stalniy

Kinda hard to contribute to this amazing and important project when the build fails :(

rinslow avatar Jul 12 '21 19:07 rinslow

Hey @stalniy could you please give a quick update on the status of making bdd-lazy-var work with jest-circus?

ignatospadov avatar Sep 23 '21 11:09 ignatospadov

No any updates guys, sorry guys.

stalniy avatar Sep 23 '21 11:09 stalniy

it seems that jest-circus exports access to its state, which potentially means I can add integration for it without changing the public interface.

stalniy avatar Jan 10 '22 22:01 stalniy

However, I still think that bdd-lazy-var need to go in the direction of being more independent from testing framework integration and being more typesafe with less hacks. This is what I plan to do in v3. This will enable this interface:

// it's a working prototype

import { lazy, clearUsedScopes } from 'bdd-lazy-var';

afterEach(() => {
  clearUsedScopes();
})

describe('suite', () => {
  let i = 0;
  const $ = lazy(b => b
    .variable('num', () => i++)
    .variable('p', 1)
    .variable('user', ({ num }) => ({ age: num }))
  )

  it('tests 1', () => {
     expect($.user.age).toEqual($.num);
  })

  it('tests 2', () => {
    expect($.user.age).toEqual($.num);
  })

  describe('nested suite', () => {
    const $1 = lazy(b => b.extends($)
      .variable('num', ({ num, p }) => p + num + 1)
    );

    it('tests', () => {
       expect($1.user.age).toEqual($1.num);
    })
  })
})

In case we want to get access to active scope in parent before/after callbacks, this interface will add few restrictions:

  1. it won't be possible to access active scope in before/after callbacks before lazy vars definition
     import { $active } from 'bdd-lazy-var/jasmine-globals';
    
     describe('test', () => {
       beforeEach(() => console.log($active.num)) // fails with error
       beforeEach(() => console.log($.num)) // works OK
    
       const $ = lazy(b => b
         .variable('num', () => 5)
         .variable('user', ({ num }) => ({ age: num }))
       )
    
       beforeEach(() => console.log($active.num)) // works OK
       beforeEach(() => console.log($.num)) // works OK
     })
    
  2. in afterAll callbacks, if lazy vars are accessed they will be re-created.
  3. Only 1 lazy block should be used inside one describe (in 99.9% this how things are done by definition but anyway)

That means, ideally we should structure our tests in such a way that we never need run side-effects based on$active scope. There is additional benefit of not using it -

stalniy avatar Jan 10 '22 22:01 stalniy

For anyone running into this error after upgrading Jest, you're seeing this because Jest 27 changed its defaults to use the Circus test runner. You can keep using bdd-lazy-var by explicitly using the older jest-jasmine2 runner. Bring in that package as a dependency:

npm i -D jest-jasmine

or

yarn add -D jest-jasmine2

Then configure Jest to use it as the runner. In your jest.config.js:

module.exports = {
	// ...
	testRunner: "jest-jasmine2",
	// ...
};

MrDOS avatar May 13 '22 10:05 MrDOS

I spent some time writing a replacement I'm calling testvar. It works with any test runner that has beforeEach and afterEach hooks. Instead of doing fancy magic with types or putting all variables into some kind of global context, each testvar is an independent function. The total code is about 300 lines of Typescript. I'll publish it to NPM if I can find the time. beforeAll and afterAll are unsupported, but could be doable if you care to -- i've never had a use for that.

Here's how it looks:

const subject = testvar(() => buildUser())
const post = testvar(() => buildPost())

it("cannot edit post", () => expect(subject().canEdit(post()).toBe(false))

describe('when user is an admin', () => {
  subject.redefine(() => 
    // calling self inside redefine is fine.
    // it will use the previous implementation.
    asAdminUser(subject())
  )
  it("can edit post", () => expect(subject().canEdit(post()).toBe(true))
})

I added an optional cleanup function, as the second argument. I use it for cleaning up resources like database connections:

const connection = testvar(
  () => new InMemoryDatabase(config()),
  // The dispose function only runs if connection() was used.
  // It will run in correct order.
  db => db.close()
)
const userRepository = testvar(() => new UserRepository(connection())

beforeEach(() => { db.migrate(); db.seed() })
it('can find a user', async () => expect(await userRepository().query().limit(1)).toBeTruthy())
afterEach(() => expect(db.inTransaction()).toBe(false))

The only configuration needed is to pass in the beforeEach and afterEach hooks, and to specify the order that afterEach runs in:

import { beforeEach, afterEach } from "@jest/globals"
export const testvar = createTestVarConstructor({
  beforeEach,
  afterEach,
  // For jasmine2, use "reverse-declaration-order"
  // jest-circus runs afterEach in declaration order, which is a bug
  // https://github.com/facebook/jest/issues/12678
  afterEachOrder: "declaration-order"
})

Here's some copypasta from my test for testvar. I removed some logging code, but hopefully it gives some more color of how it's used:

describe("testvar", () => {
	const order: string[] = []

	describe("example test suite, for testing ordering", () => {
		const outer1 = testvar(() => "outer1")
		const outer2 = testvar(() => "outer2")

		it("uses outer vars", () => {
			order.push("start:uses outer vars")
			outer1()
			outer1()
			outer2()
			outer2()
			order.push("end:uses outer vars")
		})

		describe("inner suite", () => {
			const inner1 = testvar(() => "inner1")
			const inner2 = testvar(() => `inner2 = (${outer1()}) + (${inner1()})`)

			it("uses inner and outer vars", () => {
				order.push("start:uses inner and outer vars")
				inner2()
				assert.strictEqual(inner1(), "inner1.redefined")
				outer1()
				outer2()
				order.push("end:uses inner and outer vars")
			})

			inner1.redefine(() => "inner1.redefined")

			describe("more redefinitions", () => {
				outer1.redefine(() => `outer1.redefined = (${outer1()})`)
				outer2.redefine(() => `outer2.redefined = (${inner2()})`)

				it("uses all the vars", () => {
					order.push("start:uses all the vars")
					assert.strictEqual(outer1(), "outer1.redefined = (outer1)")
					assert.strictEqual(
						outer2(),
						"outer2.redefined = (inner2 = (outer1.redefined = (outer1) + (inner1.redefined)"
					)
					assert.strictEqual(inner1(), "inner1.redefined")
					assert.strictEqual(
						inner2(),
						"inner2 = (outer1.redefined = (outer1) + (inner1.redefined)"
					)
					order.push("end:uses all the vars")
				})
			})
		})
	})

	afterAll(() => {
		const actual = order.join("\n")
		const expected = `
start:uses outer vars
create:outer1
create:outer2
end:uses outer vars
dispose:outer2
dispose:outer1
start:uses inner and outer vars
create:outer1
create:inner1.redefined
create:inner2 = (outer1) + (inner1.redefined)
create:outer2
end:uses inner and outer vars
dispose:outer2
dispose:inner2 = (outer1) + (inner1.redefined)
dispose:inner1.redefined
dispose:outer1
start:uses all the vars
create:outer1
create:outer1.redefined = (outer1)
create:inner1.redefined
create:inner2 = (outer1.redefined = (outer1) + (inner1.redefined)
create:outer2.redefined = (inner2 = (outer1.redefined = (outer1) + (inner1.redefined)
end:uses all the vars
dispose:outer2.redefined = (inner2 = (outer1.redefined = (outer1) + (inner1.redefined)
dispose:inner2 = (outer1.redefined = (outer1) + (inner1.redefined)
dispose:inner1.redefined
dispose:outer1.redefined = (outer1)
dispose:outer1
		`.trim()
		assert.strictEqual(actual, expected, "has correct total order")
	})
})

justjake avatar May 20 '22 01:05 justjake

If your test runner like jest-circus calls afterEach in declaration order (it should be called in reverse declaration order), you can invert using code like this: https://github.com/facebook/jest/issues/12678#issuecomment-1131899742

justjake avatar May 20 '22 01:05 justjake