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

Adding 'before', 'after', 'beforeEach', 'afterEach', functions to the Test Runner #43403

Closed
yaircohendev opened this issue Jun 13, 2022 · 12 comments
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.

Comments

@yaircohendev
Copy link

What is the problem this feature will solve?

Ability to write preparation and cleanup code

What is the feature you are proposing to solve the problem?

Hey,

I'd like to work on adding before, after, beforeEach, afterEach functions to the test runner.
@benjamingr
@cjihrig

What alternatives have you considered?

No response

@yaircohendev yaircohendev added the feature request Issues that request new features to be added to Node.js. label Jun 13, 2022
@VoltrexKeyva VoltrexKeyva added the test_runner Issues and PRs related to the test runner subsystem. label Jun 13, 2022
@benjamingr
Copy link
Member

Yay 🔥

@MoLow
Copy link
Member

MoLow commented Jun 29, 2022

Hi @yaircohendev where does this stand?

@yaircohendev
Copy link
Author

yaircohendev commented Jul 9, 2022

Hey @MoLow I wrote the initial code for this but having problems with debugging locally, any chance to setup a quick call about this?
Sorry for late response btw, was not getting notifications and only saw this now.

@MoLow
Copy link
Member

MoLow commented Jul 9, 2022

Hey @MoLow I wrote the initial code for this but having problems with debugging locally, any chance to setup a quick call about this? Sorry for late response btw, was not getting notifications and only saw this now.

Sure I assume we are both in Israel so we can work on this together. Ping me

@yaircohendev yaircohendev closed this as not planned Won't fix, can't repro, duplicate, stale Jul 21, 2022
@MoLow MoLow reopened this Jul 21, 2022
@9ssi7
Copy link

9ssi7 commented Jul 30, 2022

right now i'm doing it like this

"use-strict";
const assert = require("node:assert");
const test = require("node:test");
const { server, app } = require("./app.js");

test("HTTP e2e Stack Testing", async (t) => {

  // beforeAll
  await t.test("create connection before tests", async () => {
    return new Promise((resolve, reject) => {
      server.onServerStarted(() => {
        resolve();
      });
      app.start();
    });
  });

  // my tests here
  t.test("any testing", () => {
      assert.strickEqual(1, 1)
  })

  // afterAll
  server.instance.close();
});

@9ssi7
Copy link

9ssi7 commented Jul 30, 2022

But this works in single file. If I use a common source from 2 different files beforeAll doesn't work probably because nodejs runs the files on the fly. They do not wait for each other. Therefore, a global beforeAll and afterAll methods must be provided. Unfortunately, this is a mandatory requirement for memory db connections and http server installations.

aduh95 pushed a commit to aduh95/node-core-test that referenced this issue Jul 30, 2022
PR-URL: nodejs/node#43730
Fixes: nodejs/node#43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
(cherry picked from commit 659dc126932f986fc33c7f1c878cb2b57a1e2fac)
danielleadams pushed a commit that referenced this issue Aug 16, 2022
PR-URL: #43730
Fixes: #43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
ruyadorno pushed a commit that referenced this issue Aug 23, 2022
PR-URL: #43730
Fixes: #43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this issue Sep 5, 2022
PR-URL: #43730
Fixes: #43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
PR-URL: nodejs#43730
Fixes: nodejs#43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this issue Oct 10, 2022
PR-URL: #43730
Fixes: #43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
juanarbol pushed a commit that referenced this issue Oct 11, 2022
PR-URL: #43730
Fixes: #43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Jan 3, 2023
PR-URL: nodejs/node#43730
Fixes: nodejs/node#43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Jan 3, 2023
PR-URL: nodejs/node#43730
Fixes: nodejs/node#43403
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@SynLocker
Copy link

any update about this?

@MoLow
Copy link
Member

MoLow commented Apr 20, 2023

@SynLocker this exists in node18 and above

@isaacgr
Copy link

isaacgr commented Aug 3, 2023

Is it expected that these run regardless of if the test is skipped? So if a test is being skipped the 'beforeEach' call still runs before that test is officially skipped.

@ericminio
Copy link

Illustrating the question from @isaacgr

The code below is green with node 18.17.1.

const { describe, it, beforeEach } = require('node:test');
const { strict: assert } = require('node:assert');

describe('skipping test', () => {
    let spy = 0;
    beforeEach(() => {
        spy += 1;
    });

    it.skip('tries to leak', () => {
        spy += 1;
    });

    it('suffers from leak', () => {
        spy += 1;
        assert.equal(spy, 3);
    });
});

@listvin
Copy link

listvin commented Nov 26, 2023

beforeEach and afterEach should not run for skipped tests. I believe it's a bug.
As mentioned by @isaacgr and illustrated by @ericminio.
Following might illustrate better.

test1 fails with assert message:

   >> beforeEach for 'test0 WILL NOT/HAS NOT RUN'
   >> afterEach for 'test0 WILL NOT/HAS NOT RUN'
   >> beforeEach for 'test1'
   >> it test1

while expected log is:

   >> beforeEach for 'test1'
   >> it test1

test itself:

const { beforeEach, test, afterEach } = require('node:test');
const assert = require('node:assert');

const log = [];
beforeEach((sc) => {
    log.push(`beforeEach for '${sc.name}'`);
});

test.skip('test0 WILL NOT/HAS NOT RUN', (sc) => {
    log.push(`it ${sc.name}`); //as expected it didn't run
});

test('test1', (sc) => {
    log.push(`it ${sc.name}`);
    assert.equal(log.length, 2, "call log:\n >> " + log.join('\n >> '));
});

afterEach((sc) => {
    log.push(`afterEach for '${sc.name}'`);
});

Node v20.10.0. Same thing happens within describe. Skipping with --test-name-pattern gives the same also.

@benjamingr, @MoLow, any comments? It it expected behaviour? Should I open a new issue or can this one be reopened?

@louwers
Copy link

louwers commented May 26, 2024

@listvin This seems to be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants