Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CITGM style ci for direct express deps #304

Open
jonchurch opened this issue Nov 13, 2024 · 3 comments
Open

CITGM style ci for direct express deps #304

jonchurch opened this issue Nov 13, 2024 · 3 comments

Comments

@jonchurch
Copy link
Member

jonchurch commented Nov 13, 2024

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
Copy link
Member Author

jonchurch commented Nov 13, 2024

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

@ljharb
Copy link
Contributor

ljharb commented Nov 13, 2024

see also https://npmjs.com/wiby

@UlisesGascon
Copy link
Member

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 ❤️.

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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants