-
-
Notifications
You must be signed in to change notification settings - Fork 191
fix: Add npm-source suite to CI pipeline and fix npm source installation recipe #292
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
base: main
Are you sure you want to change the base?
Conversation
|
@damacus 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
Co-authored-by: damacus <[email protected]>
|
@copilot Investigqte and fix errors woth newly enabled suite |
…m names - Changed npm_from_source.rb to use :install action instead of :install_with_make - Added has_binaries property to specify npm and npx binaries for symlinking - Fixed CI platform names to match kitchen.dokken.yml (debian-12 only, ubuntu-22.04, ubuntu-24.04) - npm doesn't have a Makefile and doesn't need compilation - it's a pure JavaScript package Co-authored-by: damacus <[email protected]>
Head branch was pushed to by a user without write access
Fixed in commits 6bb6446 and 76b07ad. Root cause: The npm_from_source recipe used Changes:
All linters and unit tests pass. CI is now running to validate the fixes. |
Slowest examplesTop 10 slowest examples (6.58 seconds, 99.86% of total time)
|
|
@copilot continue iterating on the issues. Once you have identified one error. Fix and iterate until kitchen test completes without error |
Test Kitchen converts dots to empty strings in instance names, so ubuntu-22.04 in kitchen.dokken.yml becomes ubuntu-2204 in instance names Co-authored-by: damacus <[email protected]>
Continuing iteration on CI failures. Fixed Ubuntu platform naming in commit 2db715a. Issue Found: Test Kitchen strips dots from platform names, so Fix Applied: Updated CI to use Now monitoring CI run to identify any remaining issues. |
The npm source installation tests were defined but not being executed in the CI pipeline, and the underlying npm_from_source recipe had a critical bug that prevented npm from being installed from source.
Issues Fixed
npm_from_source Recipe Bug: The recipe used
:install_with_makeaction which attempted to runmakeandmake install, but npm is a pure JavaScript package without a Makefile. This caused all npm source installations to fail.Missing CI Test Suite: The
npm_sourcetest suite exists inkitchen.ymlwith corresponding test files in/test/integration/npm_source/, but thenpm-sourceentry was missing from the GitHub Actions CI matrix.Platform Configuration Mismatches: CI platform names didn't match Test Kitchen's instance naming convention (debian-11 removed, ubuntu naming corrected to match Test Kitchen's format).
Changes Made
:install_with_maketo:installaction withhas_binaries ['bin/npm', 'bin/npx']to properly extract and symlink npm binaries"npm-source"to the CI test suite matrix in.github/workflows/ci.ymlamazonlinux-2023andfedora-latestplatforms for the npm-source suite, matching the exclusions for the nodejs source suite since npm source installation depends on nodejs source installationdebian-12,ubuntu-2204, andubuntu-2404to match Test Kitchen's instance naming convention (Test Kitchen strips dots from platform names when creating instances)Test Coverage
The npm-source suite now tests npm source installation across 7 Linux distributions:
The existing test files verify that:
npm -v)npx -v)/usr/local/bin/Validation
This ensures npm source installation is properly tested and functional alongside the existing npm embedded installation tests.
Fixes #232.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.