search

interledgerjs / five-bells-integration-test

A module to help with testing Five Bells components against each other
Other
2 stars 3 forks source link

feat: enables integration tests for five-bells-shared and ilp-routing #58

Closed dappelt closed 7 years ago

justmoon commented 7 years ago

Hmm, this doesn't seem to work as intended. It's still installing different versions of those libraries.

five-bells-integration-test-instance@ /opt/workspace/interledgerjs/five-bells-integration-test/integration-test
├─┬ ilp-connector@13.0.1 (git://github.com/interledgerjs/ilp-connector.git#7c1a849c09494ceeb9d3520e8892390848d5cfc6)
│ └── ilp-routing@5.3.0 
├─┬ ilp-kit@1.0.0 (git://github.com/interledgerjs/ilp-kit.git#679936bdb72118168a59b17311c09291a9b08373)
│ ├─┬ ilp-connector@12.11.2
│ │ └── ilp-routing@5.3.0 
│ ├── UNMET PEER DEPENDENCY react@15.4.2
│ └── UNMET PEER DEPENDENCY supertest@*
└── ilp-routing@5.4.0  (git://github.com/interledgerjs/ilp-routing.git#acc8136467d700cd4cdd45ffdb5e278973ebb4e0)

The fix could be after npm install to go in and find and delete all folders matching node_modules/**/node_modules/[module] where ** is an arbitrary string (which may contain additional levels of node_modules) and [module] is one of the modules listed in the DependencyManager options. Could be done using a module like glob.

dappelt commented 7 years ago

@justmoon You are right! In fact, the problem is not only limited to the newly added modules ilp-routing and five-bells-shared, but also applies to other dependencies (i.e. ilp-core, ilp, ilp-plugin-bells).

I implemented your suggested fix and it seems to work. However, the more I am thinking about it I am not sure if I like this solution. With this fix, we are removing the dependency version as defined in the package.json and replace it with the latest version. On one hand, we make sure that all the latest versions of our modules work together, which is good. On the other hand, we have no guarantees that the tests would pass if the module is executed with the dependency version as defined in the package.json and, hence, if we would npm install a module the ordinary way it might not work.

Instead of "automatically" changing dependency versions, should we instead enforce that the referenced versions of our modules in package.json are the latest version? This way we would know that 1) the latest version of our modules are compatible and 2) the version tested in integration testing is the same as the one obtained by an ordinary npm install.

cc @emschwartz

justmoon commented 7 years ago

Instead of "automatically" changing dependency versions, should we instead enforce that the referenced versions of our modules in package.json are the latest version?

I think we have to achieve several goals:

  1. The master branches for all of our modules should pass integration tests
  2. The latest version on NPM should match the master branch for each module
  3. ILP Kit should come with the latest versions of everything

The way I'd try to achieve each is:

  1. This is what five-bells-integration-test-loader should worry about
  2. Perhaps we should deploy semantic-release for all modules
  3. This is probably the most difficult; we could add some sort of release process for ilp-kit, where we develop on a develop branch, use yarn or npm shrinkwrap to guarantee a specific set of modules and whenever we update master we run a test that ensures all modules are the latest version

@dappelt @emschwartz @vhpoet Thoughts?

dappelt commented 7 years ago

This is what five-bells-integration-test-loader should worry about

Agreed. The test loader could check that the package.json of the tested module references the latest ilp/five-bells modules. If not, we could let the build fail or give a warning.

Perhaps we should deploy semantic-release for all modules

I had a quick look at the readme, semantic-release might indeed be useful.