JamieMason / shrinkpack

Fast, resilient, reproducible builds with npm install.
https://www.npmjs.com/package/shrinkpack
MIT License
793 stars 38 forks source link

Regression in upstream sinopia Docker container #62

Closed JamieMason closed 8 years ago

JamieMason commented 8 years ago

Created a new issue to track the following.

Hoping you could help me out @5id, I'm trying to set the tests up on Travis but am running into some trouble with what I think might be the sinopia stuff.

I have a failing build output here https://travis-ci.org/JamieMason/shrinkpack/builds/147702971 – could you take a look for me please?

Thanks.

– JamieMason https://github.com/JamieMason/shrinkpack/pull/47#issuecomment-235532761


The changes are in fe6a652

– JamieMason https://github.com/JamieMason/shrinkpack/pull/47#issuecomment-235532940


It's not Travis actually, I'm having the same issues locally where the sinopia server can't be hit;

npm info attempt registry request try #1 at 3:45:46 PM
npm http request PUT http://secure_registry:4873/-/user/org.couchdb.user:shrinkpack
npm info retry will retry, error on last attempt: Error: getaddrinfo ENOTFOUND secure_registry secure_registry:4873

I'm using these versions of docker;

$ docker -v
Docker version 1.12.0, build 8eab29e

$ docker-compose -v
docker-compose version 1.8.0, build f3628c7

Could it be that something has changed in how the compose file should be written in these newer versions of docker? Any pointers would be a huge help @5id, thanks. @5id

– JamieMason https://github.com/JamieMason/shrinkpack/pull/47#issuecomment-236623212


@JamieMason it looks like there was a change in the latest Sinopia Dockerfile that we were using. In docker-compose.yml, we should change image: rnbwd/sinopia to be image: rnbwd/sinopia:stable. I'm seeing another test issue arise after fixing this that I'll look in to tomorrow.

For your own future reference, if you run docker-compose up in the package directory, it will start and show you the output for both npm & test containers (which is where I saw the NPM server exiting straight away).

– 5id https://github.com/JamieMason/shrinkpack/pull/47#issuecomment-236895153


JamieMason commented 8 years ago

Trying to fix this but getting nowhere, my knowledge of both Docker and Sinopia aren't great (on the positive side it's a good chance to learn).

Is there a reason why rnbwd/sinopia (10k installs) was used over keyvanfatehi/sinopia (100k installs)? Are there some modifications in that fork we need?

The commit history of RnbWd/sinopia-docker really puts me off and also it looks like they might be taking it off in some other direction, although it's not really clear.

I think it might be worth us creating a container of our own on docker hub so we have it under our own control. If you have a few minutes @5id to point me in the right direction I'd be happy to try and do this myself, but I have to admit as-is I'm really struggling with it.

I need to get this fixed before I can cut another release, as these are the only functional tests we have.

Thanks.

DrewML commented 8 years ago

@JamieMason I'm probably missing a ton of context here, so apologies for my ignorance in advance :).

What's the reason for using a Docker container for the tests? Looking through the commit history, I'm inferring that it may be specifically for enabling testing against multiple versions of node/npm - is that right?

I ask because TravisCI (which it looks like you're trying to use) comes with nvm installed on it to ease testing against different versions of node. It may reduce some complexity in the testing setup if it's possible to remove docker from the picture.

JamieMason commented 8 years ago

I'm inferring that it may be specifically for enabling testing against multiple versions of node/npm - is that right?

That's basically it yeah, the goal was to be able to try and reproduce any issues that get reported, by taking the package.json and node/npm pairing. Sinopia was then added to the mix so that we could test scoped packages that require authentication.

I ask because TravisCI (which it looks like you're trying to use) comes with nvm installed on it to ease testing against different versions of node. It may reduce some complexity in the testing setup if it's possible to remove docker from the picture.

You might have an idea there, I'll give it some thought, thanks.

DrewML commented 8 years ago

I'll give it some thought, thanks.

Cool. Happy to contribute if it's something you want to explore further.

You can also tell Travis to spin up a separate container for each node version you're testing against. Here is an example config from another project

JamieMason commented 8 years ago

Thanks @DrewML, let's take a look. As you say, I think the docker component may not really be needed. All that's required is that we can set the job to run on a given node/npm pairing. I can be DM'd on Gitter if needed, cheers – much appreciated.

JamieMason commented 8 years ago

By the way, if you clone, develop is the newest branch.

JamieMason commented 8 years ago

tech-task/travis is up to date with develop now too.

DrewML commented 8 years ago

Awesome! I've got lots of packing/moving this weekend, but I'm going to try and squeeze this in during some of my rest breaks :)

DrewML commented 8 years ago

@JamieMason I started working on this a bit yesterday/today. Here is my progress so far - feedback welcome.

  1. Main test file. The test cases I still need to complete are marked todo at the bottom of the file
  2. Test Helpers

A few comments:

  1. My general idea is that we would specify multiple values for the node_js key in travis.yml, like the example I linked to in my prior comment. Then, for the test command, we would do multiple entries for NPM_VERSION=3.10.6 npm run test-experimental, replacing the version for each one we want to test. That way, every node version we add gets tested across every node specified.
  2. I'm not sure that, in it's current form, sinopia is adding any value. As far as I can tell from reading the existing test file, it doesn't look like any private packages are being tested - just scoped packages (which can be public). I did work on a setup for Sinopia, but it may be worthwhile to scrap it if it's not strictly necessary.
  3. I pulled in a JS test framework and switched over from the shell scripting. My thought process was that it might be nice to test things such as "all resolve entries point to a local tarball`. This would be a bit difficult without access to a JSON parser.

My TODO for now is:

  1. Finish up existing test cases
  2. Setup a run in TravisCI under my fork to make sure everything works as expected.
JamieMason commented 8 years ago

@DrewML's improvements released in 0.16.1 🎆