This repository has been archived by the owner on May 6, 2020. It is now read-only.
A big revamp #84
Merged
A big revamp #84
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smathot
reviewed
Jul 12, 2018
There was a problem hiding this comment.
I can succesfully build. However, the unit test fails. This is with a clean npm install on Ubuntu 18.04.
osweb (revamp)$ npm test
> [email protected] test /home/sebastiaan/git/osweb
> jest
FAIL src/js/tests/classes/syntax.test.js
● Test suite failed to run
TypeError: Cannot set property 'fillStyle' of null
> 1 | import * as PIXI from 'pixi.js'
| ^
2 | import {
3 | constants
4 | } from './constants.js'
at createWhiteTexture (node_modules/pixi.js/src/core/textures/Texture.js:638:5)
at Object.<anonymous> (node_modules/pixi.js/src/core/textures/Texture.js:670:17)
at Object.<anonymous> (node_modules/pixi.js/src/core/sprites/Sprite.js:4:1)
at Object.<anonymous> (node_modules/pixi.js/lib/core/index.js:93:15)
at Object.<anonymous> (node_modules/pixi.js/lib/index.js:18:13)
at Object.<anonymous> (src/js/osweb/system/runner.js:1:1)
at Object.<anonymous> (src/js/osweb/index.js:15:1)
at Object.<anonymous> (src/js/tests/classes/syntax.test.js:4:1)
PASS src/js/tests/system/transfer.test.js
PASS src/js/tests/system/util.test.js
FAIL src/js/tests/backends/canvas.test.js
● Test suite failed to run
TypeError: Cannot set property 'fillStyle' of null
> 1 | import * as PIXI from 'pixi.js'
| ^
2 | import Styles from './styles.js'
3 |
4 | /** Class representing a drawing canvas. */
at createWhiteTexture (node_modules/pixi.js/src/core/textures/Texture.js:638:5)
at Object.<anonymous> (node_modules/pixi.js/src/core/textures/Texture.js:670:17)
at Object.<anonymous> (node_modules/pixi.js/src/core/sprites/Sprite.js:4:1)
at Object.<anonymous> (node_modules/pixi.js/lib/core/index.js:93:15)
at Object.<anonymous> (node_modules/pixi.js/lib/index.js:18:13)
at Object.<anonymous> (src/js/osweb/backends/canvas.js:1:1)
at Object.<anonymous> (src/js/tests/backends/canvas.test.js:3:1)
Test Suites: 2 failed, 2 passed, 4 total
Tests: 9 passed, 9 total
Snapshots: 0 total
Time: 10.744s
Ran all test suites.
npm ERR! Test failed. See above for more details.
|
Strange! This appears to be related to the PIXI library. |
|
It appears to be caused by a missing canvas-prebuilt module, but it is present among the devDependencies in package.json so it should have been installed. Have you tried executing |
|
I'm running the standard node packaged of Ubuntu 18.04. I indeed first removed |
|
Perfect! Feel free to merge the PR then whenever you see fit. |
|
No sorry, that's not what I meant. The |
|
Ah darn. Can you provide the output of Another thing you could try: the line suggests node-gyp needs to be installed globally. Canvas-prebuilt uses gyp to compile so maybe/hopefully could do something. You do need to perform a fresh install again after this with I also work with Ubuntu 18.04 and have no problems with this so it's hard for me to reproduce. I hope to help you solve this by just sharing my ideas like this. |
|
If I first run For future reference: What version of node are you running on Ubuntu? |
|
Still, I'd like to have this fixed for you too. It would be great if you are also able to review unit tests and see if they make sense, so let's have another go at this when you have some spare time. I'm also using node v8 on Ubuntu, but I don't know the minor version (working on my mac atm), although I don't think that matters. I have installed it from the official node.js repos though, and not the Ubuntu ones. Instructions for installing node from the official repos can be found on https://nodejs.org/en/download/package-manager/#debian-and-ubuntu-based-linux-distributions, if you're interested. Their docs also suggest to install the build-essentials package with Maybe this package is not present on your installation which is causing the node-gyp installation problem? The latest LTS is version 10 by the way, which has a lot of improvements and optimisations so I'm considering an upgrade soon. The Travis unit tests on Github both run and pass on node v8 and v10, so it's really weird that it doesn't work for you. |
|
@smathot that error you have there is related to a virus in [email protected] ... See eslint/eslint-scope#39 |
|
Wow. That's quite shocking actually.... Thanks for the heads up! |
|
@smathot I get the same error message when running the tests as you got. I don't know why, but at least now I can try to tackle it. Will keep you posted! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's been quite a while since the last PR, so a lot has happened. My main focus has been on imposing a structure on how we code, and make sure this is (gently) enforced. I also reworked the transfer module to let it use pako instead of gzip to decompress osexp files (pako is much more npm/node friendly). Lastly, I have updated all dependencies, so deleting the
node_modulesfolder completely and reinstalling everything withnpm installis highly recommended! To summarize