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

test_runner: enable in-source testing #46638

Closed
wants to merge 2 commits into from

Conversation

debadree25
Copy link
Member

@debadree25 debadree25 commented Feb 13, 2023

Just opening a draft PR to ask for feedback plus seek some help, have been trying implement in-source testing as per the linked issue, and would like to know if

a) This is the correct way to make the change
b) Also it seems that --test option value is not getting recognized somehow when using it within lib/internal/modules/esm/initialize_import_meta.js which is quite weird if anyone can explain this would be very helpful!!

Also most importantly would need discussion on whether this feature even is acceptable, the discussion on the original issue stopped some time back so hoping this one can continue the conversation

Fixes: #45771

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/modules

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Feb 13, 2023
@debadree25
Copy link
Member Author

cc @MoLow

@debadree25

This comment was marked as outdated.

@debadree25
Copy link
Member Author

Ok have been able to work around the limitation of --test not coming for some reason with a new flag --test-in-source so something like

import assert from 'node:assert';
if (import.meta.test) {
  const { describe, it } = import.meta.test;
  describe('Test Suite', () => {
    it('Test Something', () => {
      assert.ok(true);
    });
  });
}

is working with node --test-in-source test.mjs

will try to work on tests now

@MoLow
Copy link
Member

MoLow commented Feb 14, 2023

as stated here, the test runner is supposed to be lean and minimal - meaning it should include features that address the common testing use cases.
I am not convinced that this feature will be broadly adopted by node projects since:

  1. this is a personal opinion but I consider in-source testing to be an anti-pattern for the majority of use-cases
  2. it is ESM only, which is not (yet?) the majority of projects. if we want such a feature it should support both ESM and CJS

@debadree25
Copy link
Member Author

Ahh understood I guess will keep it closed for now, will reopen if there is significant interest

@debadree25 debadree25 closed this Feb 14, 2023
@cjihrig
Copy link
Contributor

cjihrig commented Feb 14, 2023

I think it's also worth noting that (if I understand the feature request correctly) this can already be done using a conditional + require() or dynamic import(). One possible example:

if (process.env.NODE_ENV === 'test') {
  const test = require('node:test');
  // test things
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test Runner: In-source testing
4 participants