Skip to content

Conversation

@takahirox
Copy link
Collaborator

@takahirox takahirox commented Dec 11, 2020

Related issue: #20795 (comment)

Background:

There are two ways to run our unit tests.

  1. Run the tests on Node.js with npm run test-unit (or other) command.
  2. Run the tests in web browser on http://localhost:8080/test/unit/UnitTests.html.

Problem:

The both run the same unit test sets. (I focus on core source code unit tests for now.) And Three.js partly has code including features Node.js doesn't natively support, for example DOM operation.

Then currently we can't add test cases using such features because the tests fail or terminate on Node.js.

Suggestion:

I'd like to suggest -webonly keyword (better name idea is welcome) to QUnit.module name for skipping such the tests on Node.js.

QUnit.config.filter can filter the tests. My idea is running the tests on Node.js excepts for the ones whose module name includes -webonly while running all the tests on Web.

https://api.qunitjs.com/config/QUnit.config/

Example:

export default QUnit.module( 'Foo', () => {
  QUnit.module('Bar', () => {
    QUnit.test('baz', ( assert ) => {
      // This test runs on both Node.js and Web.
    });
  });
  QUnit.module('Bar-webonly', () => {
    QUnit.test('baz', ( assert ) => {
      // This test runs only on Web.
    });
  });
});

I added WebGLRenderer instantiation test as an example in this PR. new WebGLRenderer() doesn't work as is on Node.js because it has a dependency with document.

The following two screenshots show that the tests pass on both Node.js and Web although the WebGLRenderer() instantiation test is added.

image

image

Benefits

We can add the tests more to the unit tests. It helps keeping Three.js functionality.

Alternative solutions

  1. Separating the unit tests which don't run on Node.js from the current unit test files. And Node.js runs the separated one while Web test runs the both.
  2. Import polyfill or libraries which emulates the features Node.js natively doesn't support for the tests. For example headless-gl for WebGL.

Probably my idea is simpler than the alternatives. Disadvantage compared to 2 is we still can't run the tests using the features Node.js doesn't natively support on Node.js

This contribution is made at San Francisco International Airport.

Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach since it's an easy solution for enabling more unit tests.

And if we come up with an alternative in the future which allows unit tests to run in the browser and node, it should be no deal to remove -webonly.

@mrdoob mrdoob added this to the r124 milestone Dec 14, 2020
@mrdoob mrdoob merged commit 465e1ab into mrdoob:dev Dec 14, 2020
@mrdoob
Copy link
Owner

mrdoob commented Dec 14, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants