From ff4ad2960355892328ec3aef50ea9fb5597c04c1 Mon Sep 17 00:00:00 2001 From: Nikolay Gagarinov Date: Thu, 10 Sep 2026 22:22:26 +0500 Subject: [PATCH 1/2] Revert "Merge pull request #42 from Hexlet/refactor/drop-package-name-check" This reverts commit 5e70401fde4387c3a7166d9cb790b9bb8f5d0402, reversing changes made to 10b3e091b7d1ebd6302a8770b8c60aa06143eb39. --- AGENTS.md | 5 +- .../package_files/correct/composer.json | 64 +++++++++++++++++++ .../package_files/correct/package.json | 29 +++++++++ .../package_files/correct/pyproject.toml | 27 ++++++++ .../package_files/wrong/composer.json | 64 +++++++++++++++++++ __fixtures__/package_files/wrong/package.json | 29 +++++++++ .../package_files/wrong/pyproject.toml | 33 ++++++++++ __tests__/packageChecker.test.js | 50 +++++++++++++++ package-lock.json | 15 +++++ package.json | 1 + src/index.js | 5 +- src/packageChecker.js | 62 ++++++++++++++++++ 12 files changed, 380 insertions(+), 4 deletions(-) create mode 100644 __fixtures__/package_files/correct/composer.json create mode 100644 __fixtures__/package_files/correct/package.json create mode 100644 __fixtures__/package_files/correct/pyproject.toml create mode 100644 __fixtures__/package_files/wrong/composer.json create mode 100644 __fixtures__/package_files/wrong/package.json create mode 100644 __fixtures__/package_files/wrong/pyproject.toml create mode 100644 __tests__/packageChecker.test.js create mode 100644 src/packageChecker.js diff --git a/AGENTS.md b/AGENTS.md index fda4293..220f723 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ Dependencies come from `make install`, and CI uses the same target. The rest of **`make setup` installs nothing.** The target reads `setup: pull setup`, so make drops the circular dependency, runs `pull`, stops there and still exits 0. -`make test` needs the fixture image that `make pull` fetches. A single file goes through jest directly: `npx jest __tests__/index.test.js`. +`make test` needs the fixture image that `make pull` fetches. A single file goes through jest directly: `npx jest __tests__/packageChecker.test.js`. The fixture image name `hexlet-project-source-ci_en` is pinned in three places that have to agree: `Makefile` (target `pull`), `server.js` (the API stub for `e2e`) and `__tests__/index.test.js` (the nock response). @@ -31,8 +31,7 @@ The fixture image name `hexlet-project-source-ci_en` is pinned in three places t - `dist/` is gitignored and stays out of git: `release.yml` builds it with `@vercel/ncc` and force-adds it onto the `release` branch. `action.yml` points at `dist/run-tests/index.js` and `dist/run-post-actions/index.js`. - Two entry points in `bin/` are the two phases of the Action: `bin/run-tests.js` (main) and `bin/run-post-actions.js` (post — finishes the check and uploads artifacts). - `src/index.js` holds the orchestration: `prepareProject()` pulls the image and extracts the project source, `check()` runs Compose, `runTests()` and `runPostActions()` talk to the Hexlet API. -- `src/routes.js` builds the API urls. -- **The action does not check the name of the student's package.** It used to, per language, and the check was dropped: the name is load-bearing only where the project's own harness resolves the package by it. That is the php library projects, whose root `composer.json` requires `hexlet/code` from a path repository — a wrong name fails `composer install` on its own. For javascript the harness `package.json` names the dependency `"@hexlet/code": "file:code"`, and npm installs a path dependency under that key whatever the package calls itself, so the import resolves either way. For python nothing reads the distribution name: the console scripts have names of their own. +- `src/routes.js` builds the API urls. `src/packageChecker.js` validates the package name of the student's project against the conventions of its language, reading `pyproject.toml`, `composer.json` or `package.json`. - `check()` calls two fixed compose service names of the project — `app` for `make setup`, then `test` — and the exit code of `test` is the verdict. Both the fixed names and the flags carry `NOTE` comments in the code; read them before changing the commands. - Tests stub the Hexlet API with a local Fastify server (`server.js`), fixtures live in `__fixtures__/`. - Artifacts of the student's tests are collected from `/tmp/artifacts/*/**` and uploaded as the `test-results` artifact. The glob starts one level down, so a file lying directly in `tmp/artifacts/` never reaches the student. diff --git a/__fixtures__/package_files/correct/composer.json b/__fixtures__/package_files/correct/composer.json new file mode 100644 index 0000000..4e23685 --- /dev/null +++ b/__fixtures__/package_files/correct/composer.json @@ -0,0 +1,64 @@ +{ + "name": "hexlet/code", + "type": "project", + "description": "Page Analyzer", + "keywords": [ + "page", + "analyzer" + ], + "license": "MIT", + "config": { + "optimize-autoloader": true, + "preferred-install": "dist", + "sort-packages": true + }, + "extra": { + "laravel": { + "dont-discover": [] + } + }, + "autoload": { + "psr-4": { + "App\\": "app/", + "Database\\Factories\\": "database/factories/", + "Database\\Seeders\\": "database/seeders/" + } + }, + "autoload-dev": { + "psr-4": { + "Tests\\": "tests/" + } + }, + "minimum-stability": "dev", + "prefer-stable": true, + "scripts": { + "post-autoload-dump": [ + "Illuminate\\Foundation\\ComposerScripts::postAutoloadDump", + "@php artisan package:discover --ansi" + ], + "post-root-package-install": [ + "@php -r \"file_exists('.env') || copy('.env.example', '.env');\"" + ], + "post-create-project-cmd": [ + "@php artisan key:generate --ansi" + ] + }, + "require": { + "doctrine/dbal": "^3.0", + "fideloper/proxy": "^4.4", + "fruitcake/laravel-cors": "^2.0", + "guzzlehttp/guzzle": "^7.2", + "imangazaliev/didom": "^1.16", + "laracasts/flash": "^3.2", + "laravel/framework": "^8.20", + "laravel/tinker": "^2.5", + "nesbot/carbon": "^2.43" + }, + "require-dev": { + "fakerphp/faker": "^1.13", + "mockery/mockery": "^1.4", + "nunomaduro/collision": "^5.1", + "phpunit/phpunit": "^9.5", + "squizlabs/php_codesniffer": "^3.5" + } +} diff --git a/__fixtures__/package_files/correct/package.json b/__fixtures__/package_files/correct/package.json new file mode 100644 index 0000000..7ca2afc --- /dev/null +++ b/__fixtures__/package_files/correct/package.json @@ -0,0 +1,29 @@ +{ + "name": "@hexlet/code", + "version": "0.0.2", + "description": "Compares two configuration files and shows a difference.", + "type": "module", + "main": "index.js", + "bin": { + "gendiff": "bin/gendiff.js" + }, + "engines": { + "node": ">=14" + }, + "scripts": { + "test": "npx jest" + }, + "author": "Hexlet", + "dependencies": { + "commander": "^6.1.0", + "js-yaml": "^3.14.0", + "lodash": "^4.17.20" + }, + "devDependencies": { + "eslint": "^7.13.0", + "eslint-config-airbnb-base": "^14.2.1", + "eslint-plugin-import": "^2.22.1", + "eslint-plugin-jest": "^24.1.3", + "jest": "^26.6.3" + } +} diff --git a/__fixtures__/package_files/correct/pyproject.toml b/__fixtures__/package_files/correct/pyproject.toml new file mode 100644 index 0000000..11690f9 --- /dev/null +++ b/__fixtures__/package_files/correct/pyproject.toml @@ -0,0 +1,27 @@ +[project] +name = "hexlet-code" +version = "0.1.0" +description = "Diff generator" +readme = "README.md" +requires-python = ">=3.12" +dependencies = [ + "pathlib>=1.0.1", + "pyyaml>=6.0.2", +] + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +packages = ["gendiff"] + +[dependency-groups] +dev = [ + "pytest-cov>=6.0.0", + "pytest>=8.3.4", + "ruff>=0.8.3", +] + +[project.scripts] +gendiff = "gendiff.scripts.gendiff:main" diff --git a/__fixtures__/package_files/wrong/composer.json b/__fixtures__/package_files/wrong/composer.json new file mode 100644 index 0000000..87ec063 --- /dev/null +++ b/__fixtures__/package_files/wrong/composer.json @@ -0,0 +1,64 @@ +{ + "name": "wrong-package-name", + "type": "project", + "description": "Page Analyzer", + "keywords": [ + "page", + "analyzer" + ], + "license": "MIT", + "config": { + "optimize-autoloader": true, + "preferred-install": "dist", + "sort-packages": true + }, + "extra": { + "laravel": { + "dont-discover": [] + } + }, + "autoload": { + "psr-4": { + "App\\": "app/", + "Database\\Factories\\": "database/factories/", + "Database\\Seeders\\": "database/seeders/" + } + }, + "autoload-dev": { + "psr-4": { + "Tests\\": "tests/" + } + }, + "minimum-stability": "dev", + "prefer-stable": true, + "scripts": { + "post-autoload-dump": [ + "Illuminate\\Foundation\\ComposerScripts::postAutoloadDump", + "@php artisan package:discover --ansi" + ], + "post-root-package-install": [ + "@php -r \"file_exists('.env') || copy('.env.example', '.env');\"" + ], + "post-create-project-cmd": [ + "@php artisan key:generate --ansi" + ] + }, + "require": { + "doctrine/dbal": "^3.0", + "fideloper/proxy": "^4.4", + "fruitcake/laravel-cors": "^2.0", + "guzzlehttp/guzzle": "^7.2", + "imangazaliev/didom": "^1.16", + "laracasts/flash": "^3.2", + "laravel/framework": "^8.20", + "laravel/tinker": "^2.5", + "nesbot/carbon": "^2.43" + }, + "require-dev": { + "fakerphp/faker": "^1.13", + "mockery/mockery": "^1.4", + "nunomaduro/collision": "^5.1", + "phpunit/phpunit": "^9.5", + "squizlabs/php_codesniffer": "^3.5" + } +} diff --git a/__fixtures__/package_files/wrong/package.json b/__fixtures__/package_files/wrong/package.json new file mode 100644 index 0000000..02450e9 --- /dev/null +++ b/__fixtures__/package_files/wrong/package.json @@ -0,0 +1,29 @@ +{ + "name": "wrong-package-name", + "version": "0.0.2", + "description": "Compares two configuration files and shows a difference.", + "type": "module", + "main": "index.js", + "bin": { + "gendiff": "bin/gendiff.js" + }, + "engines": { + "node": ">=14" + }, + "scripts": { + "test": "npx jest" + }, + "author": "Hexlet", + "dependencies": { + "commander": "^6.1.0", + "js-yaml": "^3.14.0", + "lodash": "^4.17.20" + }, + "devDependencies": { + "eslint": "^7.13.0", + "eslint-config-airbnb-base": "^14.2.1", + "eslint-plugin-import": "^2.22.1", + "eslint-plugin-jest": "^24.1.3", + "jest": "^26.6.3" + } +} diff --git a/__fixtures__/package_files/wrong/pyproject.toml b/__fixtures__/package_files/wrong/pyproject.toml new file mode 100644 index 0000000..77cc2a4 --- /dev/null +++ b/__fixtures__/package_files/wrong/pyproject.toml @@ -0,0 +1,33 @@ +[tool.poetry] +name = "wrong-package-name" +version = "0.1.0" +description = "Task manager" +authors = ["Hexlet team "] +license = "MIT" +readme = "README.md" +homepage = "https://hexlet.io" + +packages = [ + { include = "task_manager" }, +] + +[tool.poetry.dependencies] +python = "^3.8" +Django = "^3.1.5" +python-dotenv = "^0.15.0" +gunicorn = "^20.0.4" +whitenoise = "^5.2.0" +django-bootstrap4 = "^2.3.1" +dj-database-url = "^0.5.0" +psycopg2-binary = "^2.8.6" +django-filter = "^2.4.0" + +[tool.poetry.dev-dependencies] +flake8 = "^3.8.4" +coverage = "^5.3.1" + +[tool.poetry.scripts] + +[build-system] +requires = ["poetry>=0.12"] +build-backend = "poetry.masonry.api" diff --git a/__tests__/packageChecker.test.js b/__tests__/packageChecker.test.js new file mode 100644 index 0000000..7bd8968 --- /dev/null +++ b/__tests__/packageChecker.test.js @@ -0,0 +1,50 @@ +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import checkPackageName from '../src/packageChecker.js'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +const verifiableProjects = ['javascript', 'php', 'python']; + +// NOTE: Some projects are not packages. +// Also, in some languages, verification is not needed, +// since the package name is verified when installing the dependencies. +const notVerifiableProjects = [ + 'html', + 'ruby', + 'java', + 'undefined-language', + null, + undefined, +]; + +const getFixturePath = (dirname) => + path.join(__dirname, '..', '__fixtures__', 'package_files', dirname); + +describe('test projects with correct package name', () => { + const codePath = getFixturePath('correct'); + + test.each(verifiableProjects)('%s', (sourceLang) => { + expect(() => checkPackageName(codePath, sourceLang)).not.toThrow(); + }); +}); + +describe('test projects with wrong package name', () => { + const codePath = getFixturePath('wrong'); + + test.each(verifiableProjects)('%s', (sourceLang) => { + expect(() => checkPackageName(codePath, sourceLang)).toThrow( + /^Package name should be .+ instead of wrong-package-name$/, + ); + }); +}); + +describe('test not verifiable projects', () => { + const codePath = getFixturePath('correct'); + + test.each(notVerifiableProjects)('%s', (sourceLang) => { + expect(() => checkPackageName(codePath, sourceLang)).not.toThrow(); + }); +}); diff --git a/package-lock.json b/package-lock.json index ea6726e..8370dd8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,6 +19,7 @@ "ansi-colors": "^4.1.3", "chalk": "^5.6.2", "clean-stack": "^6.0.0", + "ini": "^6.0.0", "js-yaml": "^4.1.1", "lodash": "^4.18.1" }, @@ -4561,6 +4562,15 @@ "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" }, + "node_modules/ini": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/ini/-/ini-6.0.0.tgz", + "integrity": "sha512-IBTdIkzZNOpqm7q3dRqJvMaldXjDHWkEDfrwGEQTs5eaQMWV+djAhR+wahyNNMAa+qpbDUhBMVt4ZKNwpPm7xQ==", + "license": "ISC", + "engines": { + "node": "^20.17.0 || >=22.9.0" + } + }, "node_modules/ipaddr.js": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-2.3.0.tgz", @@ -10641,6 +10651,11 @@ "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.4.tgz", "integrity": "sha512-k/vGaX4/Yla3WzyMCvTQOXYeIHvqOKtnqBduzTHpzpQZzAskKMhZ2K+EnBiSM9zGSoIFeMpXKxa4dYeZIQqewQ==" }, + "ini": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/ini/-/ini-6.0.0.tgz", + "integrity": "sha512-IBTdIkzZNOpqm7q3dRqJvMaldXjDHWkEDfrwGEQTs5eaQMWV+djAhR+wahyNNMAa+qpbDUhBMVt4ZKNwpPm7xQ==" + }, "ipaddr.js": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-2.3.0.tgz", diff --git a/package.json b/package.json index 01aa70d..d725132 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "ansi-colors": "^4.1.3", "chalk": "^5.6.2", "clean-stack": "^6.0.0", + "ini": "^6.0.0", "js-yaml": "^4.1.1", "lodash": "^4.18.1" }, diff --git a/src/index.js b/src/index.js index 205abdc..5341600 100644 --- a/src/index.js +++ b/src/index.js @@ -13,6 +13,7 @@ import { HttpClient } from '@actions/http-client'; import * as io from '@actions/io'; import colors from 'ansi-colors'; import yaml from 'js-yaml'; +import checkPackageName from './packageChecker.js'; import buildRoutes from './routes.js'; const uploadArtifacts = async (diffpath) => { @@ -134,7 +135,9 @@ const prepareProject = async (options) => { }); }; -const check = async ({ projectSourcePath }) => { +const check = async ({ projectSourcePath, codePath, projectMember }) => { + const sourceLang = projectMember.project.language; + checkPackageName(codePath, sourceLang); const options = { cwd: projectSourcePath }; // NOTE: -f docker-compose.yml is required: the project image also carries // docker-compose.override.yml, which switches app to its dev command. diff --git a/src/packageChecker.js b/src/packageChecker.js new file mode 100644 index 0000000..d7997e8 --- /dev/null +++ b/src/packageChecker.js @@ -0,0 +1,62 @@ +// @ts-check + +import fs from 'node:fs'; +import path from 'node:path'; +import ini from 'ini'; + +// import yaml from 'js-yaml'; +// import _ from 'lodash'; + +const parsers = { + json: JSON.parse, + toml: ini.parse, + // yml: yaml.load, +}; + +const getFullPath = (dirpath, filename) => path.resolve(dirpath, filename); +const getFormat = (filepath) => path.extname(filepath).slice(1); +const parse = (content, format) => parsers[format](content); +const getData = (filepath) => + parse(fs.readFileSync(filepath, 'utf-8'), getFormat(filepath)); + +const mapping = { + python: { + expectedPackageName: 'hexlet-code', + getPackageName: (codePath) => { + const data = getData(getFullPath(codePath, 'pyproject.toml')); + + return data.tool?.poetry?.name || data.project.name; + }, + }, + php: { + expectedPackageName: 'hexlet/code', + getPackageName: (codePath) => + getData(getFullPath(codePath, 'composer.json')).name, + }, + javascript: { + expectedPackageName: '@hexlet/code', + getPackageName: (codePath) => + getData(getFullPath(codePath, 'package.json')).name, + }, +}; + +const checkPackageName = (codePath, sourceLang) => { + const props = mapping[sourceLang]; + + // NOTE: If the properties for checking the current project + // is not found, skip the check. + if (!props) { + return; + } + + const { expectedPackageName, getPackageName } = props; + const packageName = getPackageName(codePath); + + if (packageName !== expectedPackageName) { + throw new Error( + `Package name should be ${expectedPackageName} instead of ${packageName}`, + ); + } +}; + +export default checkPackageName; From 4592eaa1c7f29883fc564ac55e544c76745461db Mon Sep 17 00:00:00 2001 From: Nikolay Gagarinov Date: Thu, 10 Sep 2026 22:23:59 +0500 Subject: [PATCH 2/2] docs(agents): record why the package name check stays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check was dropped in #42 as redundant with the harness resolving the student package by name, and reverted. The note keeps the measurements that settle it, so the argument does not have to be rediscovered: php and python libraries do resolve by name and fail on a mismatch, javascript does not — npm installs a path dependency under the key of the dependency, whatever the package calls itself. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 220f723..e45b5aa 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,6 +32,8 @@ The fixture image name `hexlet-project-source-ci_en` is pinned in three places t - Two entry points in `bin/` are the two phases of the Action: `bin/run-tests.js` (main) and `bin/run-post-actions.js` (post — finishes the check and uploads artifacts). - `src/index.js` holds the orchestration: `prepareProject()` pulls the image and extracts the project source, `check()` runs Compose, `runTests()` and `runPostActions()` talk to the Hexlet API. - `src/routes.js` builds the API urls. `src/packageChecker.js` validates the package name of the student's project against the conventions of its language, reading `pyproject.toml`, `composer.json` or `package.json`. + + **Do not drop this check as redundant.** It was dropped once, in #42, on the argument that the harness of every project already resolves the student package by name — and reverted. The argument holds for php (`composer.json` requires `hexlet/code` from a path repository) and for python libraries (`[tool.uv.sources]` declares `hexlet-code = { path = "code" }`, and uv refuses a mismatch with `Package metadata name … does not match given name`). It does not hold for javascript: the harness names the dependency `"@hexlet/code": "file:code"`, and npm installs a path dependency under that key whatever the package calls itself — verified on both `install` and `ci`, a package named `wrong-name` installs and imports as `@hexlet/code`. There this check is the only enforcement. Where a resolver does fail, it fails inside `make setup`, so its message lands in the middle of the log while the annotation on the run still reads "The tests have failed". - `check()` calls two fixed compose service names of the project — `app` for `make setup`, then `test` — and the exit code of `test` is the verdict. Both the fixed names and the flags carry `NOTE` comments in the code; read them before changing the commands. - Tests stub the Hexlet API with a local Fastify server (`server.js`), fixtures live in `__fixtures__/`. - Artifacts of the student's tests are collected from `/tmp/artifacts/*/**` and uploaded as the `test-results` artifact. The glob starts one level down, so a file lying directly in `tmp/artifacts/` never reaches the student.