discussions icon indicating copy to clipboard operation
discussions copied to clipboard

CITGM style ci for direct express deps

Open jonchurch opened this issue 1 year ago • 4 comments

Have we discussed this before, officially?

CI job in direct dep repos we own which run express tests against changes to the package.

(CITGM - Canary In The Gold Mine)

Proposal

In direct express dependencies, run a CI job on PR and push to pull down express, npm link/install the local package, run tests.

Benefits

  • Early detection of breaking changes
  • Reduce the manual labor of checking if a package change impacts express functionality
  • Could automatically alert the TC to especially impactful changes we should pay attention to

more words

Not as a blocker on merge of course, but as a quick heads up to signal if a change would break something in express. For both the submitter and the reviewer.

This style of CI is new to me. Its unclear how/if we "get back to green". Specifically, ci that is always red isnt useful. We can code for that though in the ci or alert system, notifying humans on newly failed tests. First pass would just be running express tests and exit 0 or 1

It would of course likely fail when working on a package major not being used by the express version being tested against.


This thought is inspired by discussions in tc chat about greater maintainer autonomy and change awareness in the individual packages.

Independent of that topic, I see value in CITGM style tests.

jonchurch avatar Nov 13 '24 08:11 jonchurch

havent tested this yet

name: Express Dependency Monitor (EDM)

on:
  push:
    branches: [ main ]
  pull_request:
    branches: [ main ]

jobs:
  test-express-with-local-package:
    runs-on: ubuntu-latest
    continue-on-error: true  # Allows the workflow to continue even if this job fails

    steps:
    - name: Checkout code
      uses: actions/checkout@v3

    - name: Set up Node.js
      uses: actions/setup-node@v3
      with:
        node-version: '18'

    - name: Install dependencies
      run: npm install

    - name: Build package
      run: npm run build --if-present

    - name: Pack local package
      run: npm pack
      shell: bash

    - name: Store package file name
      id: packfile
      run: |
        package_name=$(jq -r .name package.json | sed 's/@//; s/\//-/')
        package_version=$(jq -r .version package.json)
        echo "packfile_name=${package_name}-${package_version}.tgz" >> $GITHUB_OUTPUT
      shell: bash

    - name: Clone Express.js
      run: git clone --depth 1 https://github.com/expressjs/express.git

    - name: Install Express.js dependencies
      run: |
        cd express
        npm install

    - name: Install local package into Express.js
      run: |
        cd express
        npm install ../${{ steps.packfile.outputs.packfile_name }}

    - name: Run Express.js tests
      run: |
        cd express
        npm test


jonchurch avatar Nov 13 '24 09:11 jonchurch

see also https://npmjs.com/wiby

ljharb avatar Nov 13 '24 21:11 ljharb

I love this idea a lot!

CITGM is currently a challenge when we do releases in Node. This will helps us to anticipate issues ahead of releases :heart:.

Aside of targeting commits from our side on > push. I will suggest to trigger this workflow based on cron jobs so we can test this against the last canary release available, etc... Otherwise our anticipation will be very limited.

Obviously we can add some notifications (issues) in case that the actual tests fails and not due binaries availability.

cc:@marco-ippolito and @rafaelgss for sure they can add much more context and ideas here :+1:

UlisesGascon avatar Nov 14 '24 11:11 UlisesGascon

How WIBY works right now, I don't think it's ideal for Express, since we would have to create a GitHub token between the repositories with write permissions. I am working to make this unnecessary, which is something I will be working on these months (see https://github.com/pkgjs/wiby/pull/154).

Here's an example of how it works right now (https://github.com/bjohansebas/body-parser/pull/1, https://github.com/bjohansebas/express/pull/1).

bjohansebas avatar Mar 03 '25 22:03 bjohansebas

In about two weeks I'm going on vacation, so I'll be able to dedicate some time to this.

bjohansebas avatar Jul 03 '25 01:07 bjohansebas