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

getHistoricalCountByArea test fails in newer node versions #20

Closed
matthewberryman opened this issue Oct 14, 2015 · 15 comments
Closed

getHistoricalCountByArea test fails in newer node versions #20

matthewberryman opened this issue Oct 14, 2015 · 15 comments
Assignees
Milestone

Comments

@matthewberryman
Copy link
Contributor

Description

This is with node v4.1.2 (latest available with homebrew brew update && brew install node) but I suspect also common to newer io.js-derived node.js versions across platforms that care about properties being read only. I suspect strict mode is enable now by default and that is throwing the error because of the way those variables are defined.

Output of npm test

52 passing (883ms)
1 failing

  1. getHistoricalCountByArea validation should call the database if parameters are valid:
    TypeError: Cannot assign to read only property 'start_time' of ale
    at chainQueries (CognicityServer.js:433:21)
    at Object.server.getCountByArea (test/testCognicityServer.js:631:4)
    at Object.CognicityServer.getHistoricalCountByArea (CognicityServer.js:452:8)
    at Context. (test/testCognicityServer.js:642:10)
@matthewberryman
Copy link
Contributor Author

I also had this happen on node v0.10.26 on jenkins as well.

@tomasholderness
Copy link
Contributor

This passes on node v0.10.40 on Travis. An environmental variable tripping us up?

@matthewberryman
Copy link
Contributor Author

Sorry, typo: I meant 0.12.6
No relevant environment variables set on the two boxes I've seen this on.

@tomasholderness
Copy link
Contributor

Confirmed, fails on node v0.12.7 in Travis

@matthewberryman
Copy link
Contributor Author

@benatwork99 please fix in master and 2.0.x

@benatwork99
Copy link
Contributor

WIll do. Looking at the code what was there seems wrong, a slight change will make it more correct. I'll also update node to v12 for cognicity-rem-server, do we want to update node to v12 for cognicity-server?

@matthewberryman
Copy link
Contributor Author

I've already bumped it up (at least in 2.0.x) for cognicity-server and cognicity-rem-server to 4.2.1 which is what we use for dev and prod.

@benatwork99
Copy link
Contributor

Ok great. I'll update the version Travis uses when I commit so we can see the test pass.

@matthewberryman
Copy link
Contributor Author

I've already done that too, and it still fails. https://travis-ci.org/smart-facility/cognicity-server/builds/92194867

@matthewberryman
Copy link
Contributor Author

See 9feeca8

@benatwork99
Copy link
Contributor

I am about to push a fix for this, I notice a build error in the Travis build (expand the 'npm install' section) https://travis-ci.org/smart-facility/cognicity-server/builds/92194867

> pg@3.6.3 install /home/travis/build/smart-facility/cognicity-server/node_modules/pg
> node-gyp rebuild || (exit 0)
make: Entering directory `/home/travis/build/smart-facility/cognicity-server/node_modules/pg/build'
  CXX(target) Release/obj.target/binding/src/binding.o
In file included from /home/travis/.node-gyp/4.2.1/include/node/node.h:42:0,
                 from ../node_modules/nan/nan.h:23,
                 from ../src/binding.cc:3:
/home/travis/.node-gyp/4.2.1/include/node/v8.h:336:1: error: expected unqualified-id before ‘using’
/home/travis/.node-gyp/4.2.1/include/node/v8.h:469:1: error: expected unqualified-id before ‘using’
/home/travis/.node-gyp/4.2.1/include/node/v8.h:852:1: error: expected unqualified-id before ‘using’
In file included from ../src/binding.cc:3:0:
../node_modules/nan/nan.h:74:9: error: ‘v8::Handle’ has not been declared
../node_modules/nan/nan.h:74:15: error: expected ‘,’ or ‘...’ before ‘<’ token
...

I see the same build error locally using node 4.x or 5.x.

I will look into what this means, it looks like a fatal build error but the build then continues.

@matthewberryman
Copy link
Contributor Author

That version of pg depends on a version of nan that has issues with newer nodejs (a common issue - see nodejs/node#2798 ). Safe to disregard that, as well as warnings from lack of postgres libraries—pg will work without native libraries; we do install the native libraries on our dev and prod boxes, see https://github.com/smart-facility/cognicity-server/blob/master/.ebextensions/packages.config for example on AWS EB.

@matthewberryman
Copy link
Contributor Author

Closed by 6d331b8

@benatwork99
Copy link
Contributor

FYI I have not fixed on master yet am about to do that now.

@matthewberryman
Copy link
Contributor Author

Sorry, caught the 2.0.x commit but forgot about master. Use closes keyword in commit message.

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

No branches or pull requests

3 participants