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

testing with the library "gl" (aka headless-gl) breaks if you have more than one test file #2029

Closed
gre opened this issue Oct 31, 2016 · 20 comments
Closed

Comments

@gre
Copy link

@gre gre commented Oct 31, 2016

What is the current behavior?

I have written 2 identical tests.

  • Each test individually passes.
  • Putting the 2 tests in a same file, passes.
  • but if each test is put in 2 files. it breaks.

Repository that reproduce the bug:

https://github.com/gre/jest-multi-tests-gl-issue

What is the expected behavior?

The tests should pass. (it passes in a same file!)

--debug

jest version = 16.0.2
test framework = jasmine2
config = {
  "rootDir": "/Users/gre/Desktop/jest-multi-tests-gl-issue",
  "name": "-Users-gre-Desktop-jest-multi-tests-gl-issue",
  "setupFiles": [],
  "testRunner": "/Users/gre/Desktop/jest-multi-tests-gl-issue/node_modules/jest-jasmine2/build/index.js",
  "scriptPreprocessor": "/Users/gre/Desktop/jest-multi-tests-gl-issue/node_modules/babel-jest/build/index.js",
  "usesBabelJest": true,
  "automock": false,
  "bail": false,
  "browser": false,
  "cacheDirectory": "/var/folders/3q/zjl78zks3bv7_8lfw8r505x40000gn/T/jest",
  "clearMocks": false,
  "coveragePathIgnorePatterns": [
    "/node_modules/"
  ],
  "coverageReporters": [
    "json",
    "text",
    "lcov",
    "clover"
  ],
  "globals": {},
  "haste": {
    "providesModuleNodeModules": []
  },
  "mocksPattern": "__mocks__",
  "moduleDirectories": [
    "node_modules"
  ],
  "moduleFileExtensions": [
    "js",
    "json",
    "jsx",
    "node"
  ],
  "moduleNameMapper": {},
  "modulePathIgnorePatterns": [],
  "noStackTrace": false,
  "notify": false,
  "preset": null,
  "preprocessorIgnorePatterns": [
    "/node_modules/"
  ],
  "resetModules": false,
  "testEnvironment": "jest-environment-jsdom",
  "testPathDirs": [
    "/Users/gre/Desktop/jest-multi-tests-gl-issue"
  ],
  "testPathIgnorePatterns": [
    "/node_modules/"
  ],
  "testRegex": "(/__tests__/.*|\\.(test|spec))\\.jsx?$",
  "testURL": "about:blank",
  "timers": "real",
  "useStderr": false,
  "verbose": null,
  "watch": false,
  "cache": true,
  "watchman": true,
  "testcheckOptions": {
    "times": 100,
    "maxSize": 200
  }
}
 PASS  ./b.test.js
 FAIL  ./a.test.js
  ● foo

    TypeError: framebufferTexture2D(GLenum, GLenum, GLenum, WebGLTexture, GLint)

      at WebGLRenderingContext.framebufferTexture2D (node_modules/gl/webgl.js:2297:11)
      at Object.<anonymous>.test (a.test.js:4:12)
      at process._tickCallback (internal/process/next_tick.js:103:7)

Test Suites: 1 failed, 1 passed, 2 total
Tests:       1 failed, 1 passed, 2 total
Snapshots:   0 total
Time:        1.435s
Ran all test suites.
@thymikee
Copy link
Collaborator

@thymikee thymikee commented Dec 8, 2016

Confirmed, this is really strange. Maybe gl has some issues with running under Node's vm?

@thymikee
Copy link
Collaborator

@thymikee thymikee commented Dec 8, 2016

Although it's most probably not Jest-related (but not 100% sure, maybe we run on some edge case here). So for now I'll close the issue but happy to keep the discussion going and reopen if necessary.

@cpojer
Copy link
Contributor

@cpojer cpojer commented Dec 12, 2016

Does it work with two files when doing jest -i? We parallelize across processes, so that may be an issue here?

@thymikee
Copy link
Collaborator

@thymikee thymikee commented Dec 12, 2016

Tested with -i, fails too.

@cpojer
Copy link
Contributor

@cpojer cpojer commented Dec 12, 2016

That's really odd then because that makes it run in process, serially. It may indeed be a gl issue in this case.

@mcrawshaw
Copy link

@mcrawshaw mcrawshaw commented Mar 8, 2017

Not sure this will have any impact, but maybe try destroying the gl context at the end of each test (plus the -i arg)...

gl.getExtension('STACKGL_destroy_context').destroy()
@gre
Copy link
Author

@gre gre commented Mar 9, 2017

I'm pretty sure my tests aren't leaking a gl context (I mean, at least in my "real" tests, not the one I showcase. but yeah, I can try to add that, i'm pretty sure it won't change)

@gre
Copy link
Author

@gre gre commented Mar 9, 2017

I've updated the showcase with the destroy() and also the latest jest version. the bug still occurs. -i does not help neither.

even tho the tests are identically, only one pass. and a test file that contains twice the test still pass. this does not make sense.

@mcrawshaw
Copy link

@mcrawshaw mcrawshaw commented Mar 9, 2017

Yeah, I worked on it all morning... It is bizarre...

I can't run the test a second time within the same Jest process instance. If I quit the jest process and start again its ok.

As you mentioned, destroying contexts doesn't help, you can actually create many, many contexts in a loop without issue.

At the moment it's working because I changed the function that tests for the type error. It's situated in node_modules/gl/webgl.js.

function checkObject (object) {
  return typeof object === 'object' ||
  !object
}

All I have done is loosen the void check from object === void 0 to !object. Now it runs, at least it can create a gl context.

More work to do tomorrow.

@pconerly
Copy link

@pconerly pconerly commented Mar 15, 2017

@mcrawshaw I made that change and my yarn tests were able to run successfully multiple times. We're not doing anything more than creating the GL context in our tests, but they were able to run multiple times.

@quasor
Copy link

@quasor quasor commented Apr 6, 2017

@thymikee given the above efforts, shall we reopen this?

@thymikee thymikee reopened this Apr 6, 2017
@thymikee
Copy link
Collaborator

@thymikee thymikee commented Apr 6, 2017

Let's keep this open until we find proper solution or fix the gl lib.

@thymikee
Copy link
Collaborator

@thymikee thymikee commented May 11, 2017

Closing due to: #3552 (as a workaround use --no-cache flag for now)

@thymikee thymikee closed this May 11, 2017
@gre
Copy link
Author

@gre gre commented May 11, 2017

interesting! thanks

@alvinometric
Copy link

@alvinometric alvinometric commented Jul 31, 2017

Hey, @mikolalysenko @mourner
Any chance you could propagate @mcrawshaw 's fix into gl so it doesn't happen again ?
Or confirm why the void check has to be so strict ?
Thanks guys :)

@mourner
Copy link

@mourner mourner commented Aug 1, 2017

@alvinsight I'm not sure about that check, but I'd welcome a PR!

@mcrawshaw
Copy link

@mcrawshaw mcrawshaw commented Aug 2, 2017

I'm not sure on the fix I mentioned above, it was something I came too after tracing the code. When I apply the fix and run it now I receive this error: THREE.WebGLShader: Shader couldn't compile.. Also, it doesn't make any sense why it works.

@gre your repo works when --no-cache is added to the args. I also don't see how this would impact the result. It's just a transpilation cache, there must be more too it.

I think we need to understand just what is going on with the --no-cache flag to work out the best approach to the problem.

@alvinometric
Copy link

@alvinometric alvinometric commented Aug 2, 2017

Ah, dang, good luck @mcrawshaw !
it's weird that THREE.WebGLShader even appears there, are you using three.js ?
three.js is weird so I wouldn't use it, all my test cases work as long as I add --no-cache and they're pretty similar to @gre 's repo.
Good luck again!

@hujiulong
Copy link

@hujiulong hujiulong commented Apr 12, 2018

@mcrawshaw @mourner @gre

I found the reason, but I don't know how to fix it. I'm not sure this is the bug of gl or jest.

In gl/webgl.js, there is such a piece of code:

var gl = nativeGL.WebGLRenderingContext.prototype
// ...
var _framebufferTexture2D = gl.framebufferTexture2D    // native function
gl.framebufferTexture2D = function framebufferTexture2D(
    target,
    attachment,
    textarget,
    texture,
    level ) {
    // ...
    if ( !checkObject( texture ) ) {
        throw new TypeError( 'framebufferTexture2D(GLenum, GLenum, GLenum, WebGLTexture, GLint)' )
    }
    // ...
}

In another place in gl/webgl.js, there is such a piece of code:

// line 
for ( var i = 0; i < ATTACHMENTS.length; ++i ) {
    _framebufferTexture2D.call(
        context,
        gl.FRAMEBUFFER,
        ATTACHMENTS[ i ],
        gl.TEXTURE_2D,
        0,    //  -> as parameter `texture`
        0 )
}

under normal circumstances, _framebufferTexture2D.call should be executed the native function, however, when running with jest, it is possible to call the overridden method(the js function)

checkObject( texture ) -> checkObject( 0 );
function checkObjectA (object) {
  return typeof object === 'object' ||
  !object
}
function checkObjectB (object) {
  return typeof object === 'object' ||
  (object === void 0)
}

checkObjectA( 0 );  // true
checkObjectB( 0 );  // false
@robertleeplummerjr
Copy link

@robertleeplummerjr robertleeplummerjr commented Sep 2, 2019

I believe I know what the issue is, it is that how the library is being instantiated. There are a lot of jit overriding happening and privatizing of the native methods. The refactor I'm working on here: stackgl/headless-gl#166 should resolve it as it no longer overwrites the methods, but just properly calls them using super.method() when overloading.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.