Skip to content

Conversation

@levilime
Copy link
Contributor

@levilime levilime commented Mar 15, 2018

The previous implementation of the Euler Copy/Clone, FromArray and Set/Get property tests gave inconsistent behavior due to the QUnit step behavior. This test is rewritten to successfully test the behavior. It is related to this issue: #13606

Because the previous implementation gave inconsistent behavior due to
QUnit step behavior.
By using verifySteps instead of expected steps from qunit.
@mrdoob
Copy link
Owner

mrdoob commented Mar 16, 2018

Make sure you use hard tabs 😇

@mrdoob mrdoob added this to the r92 milestone Mar 16, 2018
@levilime
Copy link
Contributor Author

Whoops 😅, fixed using hard tabs!

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 16, 2018

I still don't understand why i don't have these fails on my machine (macOS). Neither in the browser (Chrome, FF) nor with node.js (v6.2.2).

@levilime
Copy link
Contributor Author

levilime commented Mar 16, 2018

@Mugen87 I wonder about this as well. There are some minor differences between our setups. I run windows 10, node.js 6.10.2. and have run the QUnit through the console with npm test and used the QUnit GUI in firefox. Is the QUnit version you use the one that is in the package.json, ^2.5.0 ^2.4.0?

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 16, 2018

I'm using qunit 2.4.0 see like defined in package.json? Where have you seen ^2.5.0?

"qunit": "^2.4.0",

@levilime
Copy link
Contributor Author

Oops sorry, I meant ^2.4.0. This behavior is with QUnit ^2.4.0

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 16, 2018

Strange 🤔 . I think the feedback of other developers might be useful here.

Whoever reads this: Please execute npm run test and then report the number of fails in this thread 😊. Thx.

@looeee
Copy link
Collaborator

looeee commented Mar 17, 2018

[email protected]:

# pass 637
# skip 0
# todo 1101
# fail 0

Run npm update

[email protected]:

# pass 634
# skip 0
# todo 1101
# fail 3
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test: `npm run build-test && qunit test/unit/three.source.unit.js`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\looee\AppData\Roaming\npm-cache\_logs\2018-03-17T01_41_54_945Z-debug.log

The failing tests give messages similar to

not ok 1738 # TODO Source > Textures > VideoTexture > isVideoTexture
  ---
  message: "everything's gonna be alright"
  severity: todo
  actual: false
  expected: true
  stack:     at Object.QUnit.todo (D:\Web\three.js\test\unit\three.source.unit.js:69559:12)
    at runTest (D:\Web\three.js\node_modules\qunit\qunit\qunit.js:1478:30)
    at Test.run (D:\Web\three.js\node_modules\qunit\qunit\qunit.js:1464:6)
    at D:\Web\three.js\node_modules\qunit\qunit\qunit.js:1676:12
    at Timeout.advance [as _onTimeout] (D:\Web\three.js\node_modules\qunit\qunit\qunit.js:1116:26)
    at ontimeout (timers.js:478:11)
  ...

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 17, 2018

When i upgrade to [email protected], i also have three fails. But these fails are fixed with @levilime changes 👍

@looeee You have also three fails like me. The last screenshot is not actually a fail. Since the test is marked as a TODO, you can ignore the log for now.

@mrdoob Since the PR is compatible with [email protected], we can merge this one!

@mrdoob mrdoob merged commit dccdd1d into mrdoob:dev Mar 21, 2018
@mrdoob
Copy link
Owner

mrdoob commented Mar 21, 2018

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.

4 participants