Skip to content
This repository has been archived by the owner on Dec 22, 2020. It is now read-only.

Allow using the local version of webpack/dependency #54

Open
DarkoKukovec opened this issue Sep 23, 2017 · 4 comments
Open

Allow using the local version of webpack/dependency #54

DarkoKukovec opened this issue Sep 23, 2017 · 4 comments

Comments

@DarkoKukovec
Copy link
Collaborator

Use the local version instead of pulling it from npm/github. This would optimise the CI installation and simplify local testing during development.

This would be defined as webpack/dependency version "@current" (to avoid any naming collisions).
TBD: Can we use npm link/symlinks or do we need to copy the code (and what to copy).

@DarkoKukovec
Copy link
Collaborator Author

I realised over the weekend this is quite important, because there is no other way to run canary on PRs

@joshwiens
Copy link
Member

A note on usability specific to how I intend to leverage this in contrib.

We have a support matrix for Node & Webpack consisting of both Node LTS version + a can fail for what will become the next LTS ( i.e. 9 at the moment ) & current Webpack, the major version prior & a can fail for the next release.

On the infrastructure side of things, we want to be able to go as wide as possible with the test matrix to keep the feedback time as low as possible.

Possibility 1

All of this will be done in CircleCI leveraging Workflows not Travis because it allows us to use our own containers for the build. Given the way Node works, we are going to want to build containers like so...

Point being, there isn't going to be a webpack install per se, it will come with the container.

Possibility 2
The other option is to have three test containers defined by NodeJS version with a global install for canary then pass in the Webpack version & execute canary from the command line.

Something to the effect of

    - <<: *test-latest
      node_js: 6
      env: WEBPACK_VERSION=latest JOB_PART=test
      script: canary --webpack=$WEBPACK_VERSION --dependency=<dependency_reference>

I'm leaning towards the second option. The docker container builds could be triggered every time a tag is cut & publish for the supported node versions.

The consuming builds would simply target the :latest for the target docker build container, keeping the updates for canary simple.

Point being, for this to be leveraged in a CI environment, we don't need to worry about local Webpack as build containers are spun up & destroyed for every run. We could use local installs as defined in possibility 1 but the container builds would have to be triggered from Webpack to keep them up to date which I would like to avoid.

The install time for webpack in the existing CI configuration is ~35 seconds, i'd prefer that over having to publish 6 containers ( node version + webpack version ) as opposed to 2 ( node version with canary ).

@DarkoKukovec
Copy link
Collaborator Author

There was never a concern about which node version will be used. I assumed the CI would handle this (e.g. with the travis node_js).

The issue here is how to use the currently cloned repo as the test basis instead of the linked one.
Example: ETWP is using the canary. In its config it's defined it should use Webpack 3 and master to test with.
However, on the ETWP side, you also need to define which git tags or branches are tested (e.g. tag 3.0.0 and master). This means that when someone makes a PR from another branch, it wont use that branch to run tests - it will only use the tag 3.0.0 and the master branch, and therefore it would be kind of useless to detect issues before they're merged.

What I was proposing here is to define a separate @current (or however we would call it) version (either for webpack or the dependency) which would use the currently cloned repo:

module.exports = {
  versions: {
    'v3.6.0': [{
      // dependency: 'https://github.com/webpack-contrib/extract-text-webpack-plugin/archive/master.tar.gz',
      dependency: '@current', // This would use the current branch/commit instead of master
      exampleDir: 'example',
      test: 'npm test',
    }],
  },
};

@joshwiens
Copy link
Member

I assumed the CI would handle this (e.g. with the travis node_js)

It handles it because I create a config to handle it so while yes, we would always want to use the local on the dependency side for testing via continuous integration, the above is another issue related to the usability of all of this when I actually have to make it run.

We have libraries with system level requirements, it's not as easy as just flipping node_js versions unfortunately, nor is it going to be done in travis anymore.

We have two different issues that need to be solved

Using the local dependency
From a canary outsider I would think it makes sense to make --dependency optional & default it to the local lib.

This should allow canary to be used from either side of the house.

You could use it on the Webpack side by defining dependency X@version leveraging the existing implementation to validate multiple versions of etwp for instance.

That change should also allow you to use it on the webpack-contrib side in a local library by simply not defining --dependency from the canary cli ( as mentioned in the above config snippet ). From the canary config file, you could go the optional route again & keep everything consistent, no dependency: will leverage the local library code.

Using canary in contrib
We now have libs with low level language requirements for validation ( and more coming ) which will be handled by custom build / test Docker containers.

The simplest way to leverage canary is going to be from the command line as a step: in all of the implemented workflows. My tentative plan right now is to have canary installed as a part of the Docker container build process & published to hub.docker.com with contrib libs consuming the aforementioned containers to execute their test suites via the canary cli which allows me to pivot off of the node version in a matrix and minimize the number of container slots consumed by any given pull request.

Extract text makes a poor demo case as it's only requirement is node & webpack

From here out, we will be using the closure plugin as our guinea pig as it's the only currently public repo that represents all of our complexities.

So let's take a look at how this is actually going to be leveraged by the org ....

At it's most complex, this is what we have to test.

  • On NodeJS 6LTS container w/ JDK9 & the canary-cli
    • Closure functional suite against Webpack v3
    • Closure functional suite against Webpack v4
  • On NodeJS 8LTS container w/ JDK9 & the canary-cli
    • Closure functional suite against Webpack v3
    • Closure functional suite against Webpack v4
    • Closure integration suite against Webpack@latest
  • On NodeJS 9.x container w/ JDK9 & the canary-cli
    • Closure functional suite against Webpack v3
    • Closure functional suite against Webpack v4
  • On NodeJS 8LTS base container
    • Closure static analysis
    • Closure nsp security checks
  • On NodeJS 8LTS base container
    • Semantic version step which cuts a git tag
    • NPM Publish step filtered on master

What that ends up looking like in the real world is this

screen shot 2017-11-14 at 8 09 54 am copy

To limit the number of container slots we use on any given build while maintaining a reasonable feedback time on PRs, each test container is going to validate library X against all the supported Webpack versions which by the time this is implemented will be ^3.1.0 || ^4.

So while the usability issues may appear to muddy the water as it relates to this specific issue, the proposed fix is directly applicable to it's implementation as it pertains to the organization that's going to be consuming it.

In short, there should be a concern about how this is consumed if you want webpack-contrib to actually use it.

@andreicek andreicek removed their assignment Nov 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants