Skip to content

Conversation

gklyne
Copy link
Contributor

@gklyne gklyne commented Feb 20, 2019

See #1110

This fix raises an error when the version information returned by running git is empty, thereby falling back to using package.json.

(I'm not very conversant with PR mechanics, so apologies if the form is wrong here.)

@gklyne
Copy link
Contributor Author

gklyne commented Feb 20, 2019

I see a Travis CI failure is reported, but I don't know how to find out what is actually failing.

@kjetilk
Copy link
Member

kjetilk commented Feb 20, 2019

Great, thanks!

So, about the mechanics, you need to submit it to the 5.0.0 branch, so rebase on release/v5.0.0. You generally do that when you start the branch, by having checked out that branch when you go git checkout -b, but it can be done afterwards too. Let me know if you don't know how.

Then, it is a good idea to run npm test before pushing, since it should have errored locally. It is just that the checking for standard syntax is rather picky... It doesn't allow single quotes...

@kjetilk kjetilk changed the base branch from master to release/v5.0.0 February 20, 2019 20:46
@kjetilk
Copy link
Member

kjetilk commented Feb 20, 2019

Pretty weird you get errors, but now Travis is happy, so I am happy :-)

I have rebased the PR, so what remains is just to resolve the conflict, which has happened in package-lock.json. Those are a nuisance, but just take what is in the release/v5.0.0 branch, should be OK.

@gklyne
Copy link
Contributor Author

gklyne commented Feb 21, 2019

I'm trying to get the tests to run in my local environment. As starting point, I got a local copy of release/v5.0.0, thus:

git checkout -t remotes/upstream/release/v5.0.0

So I have:

nerthus:node-solid-server graham$ git remote -v
origin	git@github.com:gklyne/node-solid-server.git (fetch)
origin	git@github.com:gklyne/node-solid-server.git (push)
upstream	git@github.com:solid/node-solid-server.git (fetch)
upstream	git@github.com:solid/node-solid-server.git (push)
nerthus:node-solid-server graham$ git status
On branch release/v5.0.0
Your branch is up-to-date with 'upstream/release/v5.0.0'.
nothing to commit, working directory clean

At this point, I figure I ought to be able tu run the tests:

nerthus:node-solid-server graham$ npm run standard

But I get a whole slew of errors reported, similar to what I was seeing before.

nerthus:node-solid-server graham$ npm --version
6.4.1
nerthus:node-solid-server graham$ node --version
v10.15.1

Here are just a few of the errors I'm seeing:

nerthus:node-solid-server graham$ npm run standard

> solid-server@5.0.0-beta.7 standard /Users/graham/workspace/github/gklyne/node-solid-server
> standard '{bin,examples,lib,test}/**/*.js'

standard: Use JavaScript Standard Style (https://standardjs.com)
standard: Run `standard --fix` to automatically fix some problems.
  /Users/graham/workspace/github/gklyne/node-solid-server/lib/common/template-utils.js:51:1: Too many blank lines at the end of file. Max of 0 allowed.
  /Users/graham/workspace/github/gklyne/node-solid-server/lib/create-app.js:135:35: Multiple spaces found before '// used for se...'.
  /Users/graham/workspace/github/gklyne/node-solid-server/lib/create-server.js:85:1: Expected indentation of 6 spaces but found 10.
  /Users/graham/workspace/github/gklyne/node-solid-server/lib/create-server.js:86:1: Expected indentation of 6 spaces but found 10.
  /Users/graham/workspace/github/gklyne/node-solid-server/lib/handlers/cors-proxy.js:29:22: Multiple spaces found before '// loopback'.
  /Users/graham/workspace/github/gklyne/node-solid-server/lib/handlers/cors-proxy.js:30:22: 

    :

  /Users/graham/workspace/github/gklyne/node-solid-server/test/unit/utils-test.js:97:83: A space is required before '}'.
  /Users/graham/workspace/github/gklyne/node-solid-server/test/unit/utils-test.js:102:55: A space is required after '{'.
  /Users/graham/workspace/github/gklyne/node-solid-server/test/unit/utils-test.js:102:101: A space is required before '}'.
  /Users/graham/workspace/github/gklyne/node-solid-server/test/utils.js:66:1: Expected indentation of 4 spaces but found 2.
  /Users/graham/workspace/github/gklyne/node-solid-server/test/utils.js:67:1: Expected indentation of 6 spaces but found 4.
  /Users/graham/workspace/github/gklyne/node-solid-server/test/utils.js:68:1: Expected indentation of 4 spaces but found 2.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! solid-server@5.0.0-beta.7 standard: `standard '{bin,examples,lib,test}/**/*.js'`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the solid-server@5.0.0-beta.7 standard script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/graham/.npm/_logs/2019-02-21T12_21_43_101Z-debug.log

@dmitrizagidulin
Copy link
Contributor

@gklyne what version of the standard npm package are you running? One issue that you could be encountering is that you might have a slightly newer (or slightly older) version of standard than what node-solid-server uses (and what gets run on Travis).

@gklyne
Copy link
Contributor Author

gklyne commented Feb 21, 2019

@dmitrizagidulin npm show standard gives me this:

nerthus:node-solid-server graham$ npm show standard

standard@12.0.1 | MIT | deps: 9 | versions: 154
JavaScript Standard Style
https://standardjs.com

(and lots of other stuff.)

@gklyne
Copy link
Contributor Author

gklyne commented Feb 21, 2019

Meanwhile I've pulled package-lock.json from the release/v5.0.0 branch to try and resolve the conflict:

    git checkout upstream/release/v5.0.0 -- package-lock.json
    git commit -m "pull package-lock.json from upstream/release/v5.0.0"
    git push

@gklyne
Copy link
Contributor Author

gklyne commented Feb 21, 2019

Hmmm.. looks like I broke it again.

@dmitrizagidulin
Copy link
Contributor

@gklyne ok, yeah, so that's what's causing the issue. You're running standard 12.0 but solid-server and travis is running standard 8.6 or something. So your version is later & stricter, and the code base has not adjusted to it.

@gklyne
Copy link
Contributor Author

gklyne commented Feb 21, 2019

@dmitrizagidulin Aha! I've downgraded to standard@8.6, and npm run standard seems to run OK.

But npm test raises a different set of errors:

 547 passing (26s)
  16 pending
  41 failing

  1) AccountManager (OIDC account creation tests)
       "before all" hook:
     Error: Expected HOSTS entries of 127.0.0.1 for nic.localhost,tim.localhost,nicola.localhost
      at Promise.all.catch (test/utils.js:67:11)

  2) Signup page where Terms & Conditions are not being enforced
       should not enforce T&C upon creating account:
     Error: getaddrinfo ENOTFOUND nicola.localhost nicola.localhost:3457
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  3) ACL with WebID+OIDC over HTTP
       "before all" hook:
     Error: Expected HOSTS entries of 127.0.0.1 for nic.localhost,tim.localhost,nicola.localhost
      at Promise.all.catch (test/utils.js:67:11)

  4) CORS Proxy
       "before all" hook:
     Error: Expected HOSTS entries of 127.0.0.1 for nic.localhost,tim.localhost,nicola.localhost
      at Promise.all.catch (test/utils.js:67:11)

  5) HTTP COPY API
       "before all" hook:
     Uncaught Error: listen EADDRINUSE: address already in use :::8443
      at Server.setupListenHandle [as _listen2] (net.js:1277:14)
      at listenInCluster (net.js:1325:12)
      at Server.listen (net.js:1412:7)
      at Context.<anonymous> (test/integration/http-copy-test.js:22:26)

  6) PATCH
       with a patch document
         with an unsupported content type
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  7) PATCH
       with a patch document
         containing invalid syntax
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  8) PATCH
       with a patch document
         without relevant patch element
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  9) PATCH
       with a patch document
         with neither insert nor delete
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  10) PATCH
       with insert
         on a non-existing file
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  11) PATCH
       with insert
         on a resource with read-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  12) PATCH
       with insert
         on a resource with append-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  13) PATCH
       with insert
         on a resource with write-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  14) PATCH
       with insert and where
         on a non-existing file
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  15) PATCH
       with insert and where
         on a resource with read-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  16) PATCH
       with insert and where
         on a resource with append-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  17) PATCH
       with insert and where
         on a resource with write-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  18) PATCH
       with insert and where
         on a resource with read-append access
           with a matching WHERE clause
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  19) PATCH
       with insert and where
         on a resource with read-append access
           with a non-matching WHERE clause
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  20) PATCH
       with insert and where
         on a resource with read-write access
           with a matching WHERE clause
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  21) PATCH
       with insert and where
         on a resource with read-write access
           with a non-matching WHERE clause
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  22) PATCH
       with delete
         on a non-existing file
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  23) PATCH
       with delete
         on a resource with read-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  24) PATCH
       with delete
         on a resource with append-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  25) PATCH
       with delete
         on a resource with write-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  26) PATCH
       with delete
         on a resource with read-append access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  27) PATCH
       with delete
         on a resource with read-write access
           with a patch for existing data
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  28) PATCH
       with delete
         on a resource with read-write access
           with a patch for non-existing data
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  29) PATCH
       with delete
         on a resource with read-write access
           with a matching WHERE clause
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  30) PATCH
       with delete
         on a resource with read-write access
           with a non-matching WHERE clause
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  31) PATCH
       deleting and inserting
         on a non-existing file
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  32) PATCH
       deleting and inserting
         on a resource with read-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  33) PATCH
       deleting and inserting
         on a resource with append-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  34) PATCH
       deleting and inserting
         on a resource with write-only access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  35) PATCH
       deleting and inserting
         on a resource with read-append access
           "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  36) PATCH
       deleting and inserting
         on a resource with read-write access
           executes deletes before inserts
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  37) PATCH
       deleting and inserting
         on a resource with read-write access
           with a patch for existing data
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  38) PATCH
       deleting and inserting
         on a resource with read-write access
           with a patch for non-existing data
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  39) PATCH
       deleting and inserting
         on a resource with read-write access
           with a matching WHERE clause
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  40) PATCH
       deleting and inserting
         on a resource with read-write access
           with a non-matching WHERE clause
             "before all" hook:
     Error: getaddrinfo ENOTFOUND tim.localhost tim.localhost:7777
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:57:26)

  41) Special handling: Root ACL does not give READ access to root
       "before all" hook:
     Error: Expected HOSTS entries of 127.0.0.1 for nic.localhost,tim.localhost,nicola.localhost
      at Promise.all.catch (test/utils.js:67:11)




=============================== Coverage summary ===============================
Statements   : 76.76% ( 2285/2977 )
Branches     : 66.76% ( 717/1074 )
Functions    : 73.23% ( 446/609 )
Lines        : 77.24% ( 2253/2917 )
================================================================================
npm ERR! code ELIFECYCLE
npm ERR! errno 41
npm ERR! solid-server@5.0.0-beta.7 nyc: `cross-env NODE_TLS_REJECT_UNAUTHORIZED=0 nyc --reporter=text-summary mocha`
npm ERR! Exit status 41
npm ERR!
npm ERR! Failed at the solid-server@5.0.0-beta.7 nyc 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!     /Users/graham/.npm/_logs/2019-02-21T23_39_26_315Z-debug.log
npm ERR! Test failed.  See above for more details.

@gklyne
Copy link
Contributor Author

gklyne commented Feb 21, 2019

Hmmm... it looks as of most of the failures may be localhost lookup problems - aren't these set up as test fixtures/mocks/or something?

@dmitrizagidulin
Copy link
Contributor

Right, so, those can't be automatically set up by the fixtures, you have to edit those yourself. See:

https://github.com/solid/node-solid-server#editing-your-local-etchosts

@gklyne
Copy link
Contributor Author

gklyne commented Feb 22, 2019

@dmitrizagidulin that did it, thanks. I also had to stop a running solid server. Now all tests pass.

It might be worth adding a link to the CONTRIBUTING file (in the bit that talks about making sure the tests pass before submitting a PR).

@gklyne
Copy link
Contributor Author

gklyne commented Feb 22, 2019

OK, I now have a branch release/v5.0.0 passing all tests and pushed to my forked repo.

It appears I can't change this pull request to change the source branch (per https://stackoverflow.com/questions/44864448/git-change-the-source-branch-of-a-pull-request), so I propose to cancel this PR and start a new one with the new branch.

@gklyne gklyne closed this Feb 22, 2019
@dmitrizagidulin
Copy link
Contributor

@gklyne You can change the source branch of a PR by doing a git rebase.

@gklyne
Copy link
Contributor Author

gklyne commented Feb 22, 2019

Ack. Thanks. I was also trying to align my branches with solid versions.

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