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

Bad test in Batched3DModel3DTileContentSpec.js #4803

Closed
mramato opened this issue Jan 3, 2017 · 3 comments
Closed

Bad test in Batched3DModel3DTileContentSpec.js #4803

mramato opened this issue Jan 3, 2017 · 3 comments

Comments

@mramato
Copy link
Contributor

mramato commented Jan 3, 2017

Unless I'm missing something, logs deprecation warning for use of BATCHID without prefixed underscore is a bad test. Here's the full test:

it('logs deprecation warning for use of BATCHID without prefixed underscore', function() {
    var deprecationWarningSpy = jasmine.createSpy(deprecationWarning);
    return Cesium3DTilesTester.loadTileset(scene, withBatchTableUrl).then(function(tileset) {
        expect(deprecationWarningSpy).toHaveBeenCalled();
        Cesium3DTilesTester.expectRenderTileset(scene, tileset);
    });
});

expect(deprecationWarningSpy).toHaveBeenCalled(); will always fail because you can't create a spy on a freestanding function like this. Basically var deprecationWarningSpy = jasmine.createSpy(deprecationWarning); is a no-op since deprecationWarningSpy is never used. When the internal Cesium code is ran, it always calls the real deprecationWarning. If you want to spy on deprecation warning, you need to create a private inner function like we do with loadWithXhr.

I'm not sure how this code got into 3d-tiles since the test never passes when ran locally (it doesn't run on the server because it's WebGL).

CC @lilleyse @pjcozzi

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 3, 2017

@lasalvavida can you look at this? It should be an easy fix.

@lasalvavida
Copy link
Contributor

Looking at this now

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 14, 2017

Fixed in #4809

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

No branches or pull requests

3 participants