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

execution order difference between jest-jasmine2 and jest-circus #8360

Closed
luxflow opened this issue Apr 22, 2019 · 8 comments · Fixed by #9965
Closed

execution order difference between jest-jasmine2 and jest-circus #8360

luxflow opened this issue Apr 22, 2019 · 8 comments · Fixed by #9965

Comments

@luxflow
Copy link

luxflow commented Apr 22, 2019

🐛 Bug Report

https://jestjs.io/docs/en/setup-teardown#order-of-execution-of-describe-and-test-blocks
manual says execution order of describe and test blocks should be

describe('outer', () => {
  console.log('describe outer-a');

  describe('describe inner 1', () => {
    console.log('describe inner 1');
    test('test 1', () => {
      console.log('test for describe inner 1');
      expect(true).toEqual(true);
    });
  });

  console.log('describe outer-b');

  test('test 1', () => {
    console.log('test for describe outer');
    expect(true).toEqual(true);
  });

  describe('describe inner 2', () => {
    console.log('describe inner 2');
    test('test for describe inner 2', () => {
      console.log('test for describe inner 2');
      expect(false).toEqual(false);
    });
  });

  console.log('describe outer-c');
});

// describe outer-a
// describe inner 1
// describe outer-b
// describe inner 2
// describe outer-c
// test for describe inner 1
// test for describe outer
// test for describe inner 2

while this is true for jest-jasmine2
In jest-circus, it is not work as expected

 PASS  test.js
  outer
    ✓ test 1 (16ms)
    describe inner 1
      ✓ test 1 (3ms)
    describe inner 2
      ✓ test for describe inner 2 (6ms)

  console.log test.js:2
    describe outer-a

  console.log test.js:5
    describe inner 1

  console.log test.js:12
    describe outer-b

  console.log test.js:20
    describe inner 2

  console.log test.js:27
    describe outer-c

  console.log test.js:15
    test for describe outer

  console.log test.js:7
    test for describe inner 1

  console.log test.js:22
    test for describe inner 2

Test Suites: 1 passed, 1 total
Tests:       3 passed, 3 total
Snapshots:   0 total
Time:        2.371s

To Reproduce

Just use same source in manual with jest-jasmine2 or jest-circus

Expected behavior

execution order must be same

Link to repl or repo (highly encouraged)

none

@jeysal
Copy link
Contributor

jeysal commented Apr 22, 2019

@SimenB Is the top to bottom order here something we want to keep in circus? Could probably be done by merging children: DescribeBlock[] and tests: TestEntry[] into children: (DescribeBlock | TestEntry)[]. retryTimes might be awkward.

@SimenB
Copy link
Member

SimenB commented Apr 22, 2019

I think the current behavior makes more sense, doesn't it? describes group/nest things, so tests nested one level down execute later.

@scotthovestadt @thymikee thoughts?

@vilicvane
Copy link

I would prefer the behavior of jasmine2. In my use case I use describe to organize larger tests that can be split, while the order still matters.

E.g.:

# 1
  # 1.1
    * 1.1.1
    * 1.1.2
  * 1.2
* 2

# for describe and * for test.

@davidje13
Copy link

I just observed this while investigating migrating a flow test (which uses --runInBand) to jest-circus.

I prefer the old test order. To provide some context, my tests look roughly like this:

describe('My site', () => {
  let driver;
  let page;
  let page2;
  let page3;

  beforeAll(() => {
    driver = /* make selenium driver */;
  });

  it('loads the welcome page', async () => {
    page = new MyPageObject(driver);
    expect(await page.getTitle()).toEqual('My site');
  });

  describe('home page', () => {
    it('says hi', async () => {
      expect(await page.getMessage()).toEqual('hi');
    });

    // ...
  });

  it('advances to page 2 when next is clicked', async () => {
    const page2 = await page.clickNext();
    expect(await page2.getTitle()).toEqual('Another page');
  });

  describe('page 2', () => {
    it('has a list', async () => {
      expect(await page2.getItems()).toEqual(['foo', 'bar']);
    });

    // ...
  });

  it('advances to page 3 when next is clicked', async () => {
    const page3 = await page2.clickNext();
    expect(await page3.getTitle()).toEqual('Final page');
  });

  describe('page 3', () => {
    it('shows details', async () => {
      expect(await page3.getDetails()).toEqual('my details');
    });

    // ...
  });
});

Each test may depend on state built-up by a previous test, and launching a new browser session and re-building that state each time is needlessly time consuming. But it's nice to still keep the details of what's being tested broken up into individual tests and describe blocks (if anything fails it's easy to see what part of the flow failed).

It is possible to re-shuffle it and describe blocks to make the ordering work, but it's a potential source of confusion for other developers. More logical if tests are run strictly in the order they appear in the file.

@cspotcode
Copy link
Contributor

I'm worried about this as well. Tests should run in the order they're declared; running all the cases first and the suites second is confusing. I can't think of any technical reasons or benefits for circus to reorder them the way it is.

@jeysal
Copy link
Contributor

jeysal commented May 2, 2020

I think we should adapt this to match Jasmine's behavior. It's more flexible. If you want to run all tests first, then all sub-describes, sure, reorder them. If you want to mix order because it makes for a better grouping, sure, we should allow that.

@jeysal jeysal added this to the Jest 26 milestone May 2, 2020
@SimenB SimenB mentioned this issue May 3, 2020
6 tasks
@jeysal jeysal self-assigned this May 3, 2020
@jeysal
Copy link
Contributor

jeysal commented May 3, 2020

Currently implementing this, slightly scared by the fact that there seems to be no existing test failing because of it :D But maybe it makes sense nobody has written this, we're probably so used to writing order-independent tests that we don't even think of any execution order as a requirement.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants