maximus/vps-health-api
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

test-curl.sh : rendre les ports des serveurs paramétrables #46

New issue
Closed
opened 2026-10-02 01:18:46 +00:00 by maximus · 0 comments
maximus commented 2026-10-02 01:18:46 +00:00
Owner
Copy link

Constat non bloquant des deux revues de la PR #45 (commentaires 17831 et 17881). Analyse du 2026-10-01 : prémisse relue dans le code et confirmée.

Contexte

test-curl.sh démarre ses deux serveurs sur 3099 et 3098, en dur (PORT=3099 l. 168 et PORT=3098 l. 183, devant node index.js). BASE_URL et NOINGEST_URL se surchargent (l. 31 et 33), mais pas le port d'écoute : les surcharger seuls fait pointer require_free, wait_for et tous les curl ailleurs que sur les serveurs démarrés. Cette surcharge n'a donc aucun usage cohérent aujourd'hui.

Depuis #41, __tests__/ci.test.js lance le script en entier 8 fois (bloc describe("test-curl.sh"), l. 224 à 402), et cinq de ces cas posent en plus un voisin ou un faux node sur 3099 ou 3098. Les deux ports sont occupés à chaque npm test, et plus seulement dans l'étape CI « Socket-level pass ».

Conséquence : deux npm test simultanés, ou un bash test-curl.sh lancé pendant un npm test, se gênent. Mesure du relecteur (commentaire 17881), 6 tours de deux passages simultanés : jamais un cas lancé contre le serveur de l'autre (require_free tient), mais au moins un des deux échoue, et les deux dans 2 tours sur 6.

Aucun autre port fixe dans la suite : les cas de boot-from-copy.sh passent déjà BOOT_PORT avec un port de freePort(), et les autres fichiers de test n'ouvrent pas de port fixe.

Décisions

  1. Ports seuls, URLs dérivées. Deux variables de port ; BASE_URL et NOINGEST_URL deviennent internes (http://localhost:<port>) et ne se lisent plus dans l'environnement. Effet de bord voulu : le contournement BASE_URL: undefined de runSocketPass (vite pose BASE_URL=/ dans le processus de test) n'a plus lieu d'être.
  2. Noms : TEST_CURL_PORT et TEST_CURL_NOINGEST_PORT, défauts 3099 et 3098. Pas PORT : un PORT hérité du shell déplacerait les serveurs en silence, et le critère « sans variable, comportement inchangé » dépendrait de l'environnement.
  3. Preuve du critère de simultanéité : un cas vitest et une mesure en PR.

Travail à faire

  • test-curl.sh : lire TEST_CURL_PORT (défaut 3099) et TEST_CURL_NOINGEST_PORT (défaut 3098), en dériver BASE_URL et NOINGEST_URL sans lecture de l'environnement, et passer ces ports aux deux lancements de node.
  • test-curl.sh : refuser avec un message sur stderr et le code 1, avant require_free, une valeur non numérique et deux ports identiques (sans ce contrôle, deux ports identiques passent require_free puis échouent sur un « exited before answering » qui ne nomme pas la cause).
  • test-curl.sh : mettre à jour l'en-tête Usage avec les deux variables.
  • ci.test.js : runSocketPass passe deux ports libres distincts au script. Les réserver ensemble (deux sondes ouvertes en même temps, puis fermées) : deux appels successifs à freePort() peuvent rendre le même port.
  • ci.test.js : les cas à voisin, à occupant non HTTP et à faux node écoutent ou filtrent sur le port passé au script ; released() vérifie ces mêmes ports ; les messages attendus sont construits depuis le port. Retirer BASE_URL: undefined et réécrire le commentaire de tête du bloc.
  • ci.test.js : nouveau cas, deux test-curl.sh lancés en même temps sur quatre ports libres, 45 passed, 0 failed des deux côtés.
  • ci.test.js : épingles sur le source pour les défauts 3099 et 3098 et pour l'absence de lecture de BASE_URL et NOINGEST_URL dans l'environnement. Par lecture du source et non par lancement, pour que npm test ne touche plus 3099 ni 3098. Le lancement sans variable reste couvert par l'étape CI « Socket-level pass ».
  • ci.test.js : cas pour les deux refus (valeur non numérique, ports identiques).
  • CLAUDE.md, section Tests : décomptes (363 cas, 33 pour ci.test.js, 8 pour le bloc test-curl.sh), phrase « Ces cas occupent 3099 et 3098 pendant npm test, et retirent BASE_URL » à retirer, paragraphe bash test-curl.sh à compléter avec les deux variables.

Fichiers concernés

  • test-curl.sh : l. 31 à 33 (URLs), l. 62 et 63 (require_free), l. 168 et 183 (lancements), en-tête.
  • __tests__/ci.test.js : freePort() l. 157, bloc describe("test-curl.sh") l. 224 à 402.
  • CLAUDE.md : section Tests.
  • .forgejo/workflows/ci.yml : inchangé. L'étape lance bash test-curl.sh sans variable, et son commentaire (« on 3099 and 3098 ») reste exact.

Surface de test

  • Tests existants : les 8 cas du bloc test-curl.sh de ci.test.js, tous à adapter. Mutations à rejouer après adaptation (table de la revue 17881) : export retiré, require_free retirés, exit 1 retirés, trap ancien.
  • Tests à ajouter : passage double simultané, épingles des défauts, deux refus.
  • Test de régression requis : oui, le cas de passage double.

Risques et limites

  • freePort() ferme sa sonde avant que le script n'écoute : entre deux npm test simultanés, un port libéré par l'un peut être rendu à l'autre. require_free ou wait_for refusent alors avec un message, sans cas lancé contre un mauvais serveur. Fenêtre étroite (ports éphémères tirés par le noyau), à quantifier par la mesure en PR et à consigner comme limite si elle se montre.
  • Les sondes écoutent sur 127.0.0.1, le serveur sur toutes les interfaces, les curl visent localhost : garder les voisins des tests sur la même forme d'écoute qu'aujourd'hui (listen(port) sans hôte).

Critères d'acceptation

  • Les deux ports de test-curl.sh se choisissent par TEST_CURL_PORT et TEST_CURL_NOINGEST_PORT, 3099 et 3098 restant les défauts.
  • BASE_URL et NOINGEST_URL de l'environnement n'ont plus d'effet sur le script.
  • ci.test.js passe des ports libres au script : plus aucun cas de npm test n'écoute sur 3099 ni 3098.
  • Deux npm test simultanés passent tous les deux : mesure sur au moins 6 tours rapportée dans la PR, et cas vitest de passage double vert.
  • bash test-curl.sh sans variable garde son comportement (45 cas sur 3099 et 3098), étape CI inchangée et verte.
  • CLAUDE.md à jour (décomptes et paragraphes cités).

Complexité estimée

Medium. Le changement du script tient en une dizaine de lignes ; l'essentiel est la reprise des 8 cas de ci.test.js et leurs mutations.

Constat non bloquant des deux revues de la PR #45 (commentaires 17831 et 17881). Analyse du 2026-10-01 : prémisse relue dans le code et confirmée. ## Contexte `test-curl.sh` démarre ses deux serveurs sur 3099 et 3098, en dur (`PORT=3099` l. 168 et `PORT=3098` l. 183, devant `node index.js`). `BASE_URL` et `NOINGEST_URL` se surchargent (l. 31 et 33), mais pas le port d'écoute : les surcharger seuls fait pointer `require_free`, `wait_for` et tous les `curl` ailleurs que sur les serveurs démarrés. Cette surcharge n'a donc aucun usage cohérent aujourd'hui. Depuis #41, `__tests__/ci.test.js` lance le script en entier 8 fois (bloc `describe("test-curl.sh")`, l. 224 à 402), et cinq de ces cas posent en plus un voisin ou un faux `node` sur 3099 ou 3098. Les deux ports sont occupés à chaque `npm test`, et plus seulement dans l'étape CI « Socket-level pass ». Conséquence : deux `npm test` simultanés, ou un `bash test-curl.sh` lancé pendant un `npm test`, se gênent. Mesure du relecteur (commentaire 17881), 6 tours de deux passages simultanés : jamais un cas lancé contre le serveur de l'autre (`require_free` tient), mais au moins un des deux échoue, et les deux dans 2 tours sur 6. Aucun autre port fixe dans la suite : les cas de `boot-from-copy.sh` passent déjà `BOOT_PORT` avec un port de `freePort()`, et les autres fichiers de test n'ouvrent pas de port fixe. ## Décisions 1. **Ports seuls, URLs dérivées.** Deux variables de port ; `BASE_URL` et `NOINGEST_URL` deviennent internes (`http://localhost:<port>`) et ne se lisent plus dans l'environnement. Effet de bord voulu : le contournement `BASE_URL: undefined` de `runSocketPass` (vite pose `BASE_URL=/` dans le processus de test) n'a plus lieu d'être. 2. **Noms : `TEST_CURL_PORT` et `TEST_CURL_NOINGEST_PORT`**, défauts 3099 et 3098. Pas `PORT` : un `PORT` hérité du shell déplacerait les serveurs en silence, et le critère « sans variable, comportement inchangé » dépendrait de l'environnement. 3. **Preuve du critère de simultanéité : un cas vitest et une mesure en PR.** ## Travail à faire - [ ] `test-curl.sh` : lire `TEST_CURL_PORT` (défaut 3099) et `TEST_CURL_NOINGEST_PORT` (défaut 3098), en dériver `BASE_URL` et `NOINGEST_URL` sans lecture de l'environnement, et passer ces ports aux deux lancements de `node`. - [ ] `test-curl.sh` : refuser avec un message sur stderr et le code 1, avant `require_free`, une valeur non numérique et deux ports identiques (sans ce contrôle, deux ports identiques passent `require_free` puis échouent sur un « exited before answering » qui ne nomme pas la cause). - [ ] `test-curl.sh` : mettre à jour l'en-tête `Usage` avec les deux variables. - [ ] `ci.test.js` : `runSocketPass` passe deux ports libres distincts au script. Les réserver ensemble (deux sondes ouvertes en même temps, puis fermées) : deux appels successifs à `freePort()` peuvent rendre le même port. - [ ] `ci.test.js` : les cas à voisin, à occupant non HTTP et à faux `node` écoutent ou filtrent sur le port passé au script ; `released()` vérifie ces mêmes ports ; les messages attendus sont construits depuis le port. Retirer `BASE_URL: undefined` et réécrire le commentaire de tête du bloc. - [ ] `ci.test.js` : nouveau cas, deux `test-curl.sh` lancés en même temps sur quatre ports libres, `45 passed, 0 failed` des deux côtés. - [ ] `ci.test.js` : épingles sur le source pour les défauts 3099 et 3098 et pour l'absence de lecture de `BASE_URL` et `NOINGEST_URL` dans l'environnement. Par lecture du source et non par lancement, pour que `npm test` ne touche plus 3099 ni 3098. Le lancement sans variable reste couvert par l'étape CI « Socket-level pass ». - [ ] `ci.test.js` : cas pour les deux refus (valeur non numérique, ports identiques). - [ ] `CLAUDE.md`, section Tests : décomptes (363 cas, 33 pour `ci.test.js`, 8 pour le bloc `test-curl.sh`), phrase « Ces cas occupent 3099 et 3098 pendant `npm test`, et retirent `BASE_URL` » à retirer, paragraphe `bash test-curl.sh` à compléter avec les deux variables. ## Fichiers concernés - `test-curl.sh` : l. 31 à 33 (URLs), l. 62 et 63 (`require_free`), l. 168 et 183 (lancements), en-tête. - `__tests__/ci.test.js` : `freePort()` l. 157, bloc `describe("test-curl.sh")` l. 224 à 402. - `CLAUDE.md` : section Tests. - `.forgejo/workflows/ci.yml` : inchangé. L'étape lance `bash test-curl.sh` sans variable, et son commentaire (« on 3099 and 3098 ») reste exact. ## Surface de test - Tests existants : les 8 cas du bloc `test-curl.sh` de `ci.test.js`, tous à adapter. Mutations à rejouer après adaptation (table de la revue 17881) : export retiré, `require_free` retirés, `exit 1` retirés, `trap` ancien. - Tests à ajouter : passage double simultané, épingles des défauts, deux refus. - Test de régression requis : oui, le cas de passage double. ## Risques et limites - `freePort()` ferme sa sonde avant que le script n'écoute : entre deux `npm test` simultanés, un port libéré par l'un peut être rendu à l'autre. `require_free` ou `wait_for` refusent alors avec un message, sans cas lancé contre un mauvais serveur. Fenêtre étroite (ports éphémères tirés par le noyau), à quantifier par la mesure en PR et à consigner comme limite si elle se montre. - Les sondes écoutent sur `127.0.0.1`, le serveur sur toutes les interfaces, les `curl` visent `localhost` : garder les voisins des tests sur la même forme d'écoute qu'aujourd'hui (`listen(port)` sans hôte). ## Critères d'acceptation - [ ] Les deux ports de `test-curl.sh` se choisissent par `TEST_CURL_PORT` et `TEST_CURL_NOINGEST_PORT`, 3099 et 3098 restant les défauts. - [ ] `BASE_URL` et `NOINGEST_URL` de l'environnement n'ont plus d'effet sur le script. - [ ] `ci.test.js` passe des ports libres au script : plus aucun cas de `npm test` n'écoute sur 3099 ni 3098. - [ ] Deux `npm test` simultanés passent tous les deux : mesure sur au moins 6 tours rapportée dans la PR, et cas vitest de passage double vert. - [ ] `bash test-curl.sh` sans variable garde son comportement (45 cas sur 3099 et 3098), étape CI inchangée et verte. - [ ] `CLAUDE.md` à jour (décomptes et paragraphes cités). ## Complexité estimée Medium. Le changement du script tient en une dizaine de lignes ; l'essentiel est la reprise des 8 cas de `ci.test.js` et leurs mutations.
maximus added the
status:triage
type:infra
source:human
 labels 2026-10-02 01:18:46 +00:00
maximus added
status:ready
 and removed
status:triage
 labels 2026-10-02 01:31:23 +00:00
maximus added
status:review
 and removed
status:ready
 labels 2026-10-02 01:42:00 +00:00
maximus added
status:approved
 and removed
status:review
 labels 2026-10-02 17:51:48 +00:00
maximus added
status:review
 and removed
status:approved
 labels 2026-10-02 18:06:16 +00:00
maximus added
status:approved
 and removed
status:review
 labels 2026-10-02 18:12:02 +00:00
maximus added
status:review
 and removed
status:approved
 labels 2026-10-02 18:15:05 +00:00
maximus added
status:approved
 and removed
status:review
 labels 2026-10-02 18:18:59 +00:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
issue-4-runtime-only-coolify-secrets
No results found.
Labels
Clear labels   
autopilot:pending-human

PR produite par /autopilot, attend review humaine (jamais mergee par le run)

  
source:analyste

Créée par le Super-Analyste

  
source:defenseur

Créée par l Escouade Défenseur

  
source:human

Créée par un humain

  
source:medic

Créée par le Medic (auto-détection)

  
status:approved

PR approuvée, prête à merger

  
status:blocked

Bloquée (question, dépendance, clarification)

  
status:in-progress

Un agent (Medic/Dev) travaille dessus

  
status:needs-clarification

Issue bloquee par une ambiguite LOW pendant un run autonome, Max doit trancher

  
status:needs-fix

Reviewer demande des corrections

  
status:ready

Analysée et documentée, prête à prendre

  
status:review

PR créée, en attente du Reviewer

  
status:triage

Nouvelle issue, pas encore analysée

  
type:bug

Correction de bug → Medic

  
type:feature

Nouvelle fonctionnalité → Dev

  
type:infra

Config, CI/CD, déploiement → Dev

  
type:refactor

Refactoring / amélioration technique → Dev

  
type:schema

Modèle de données / migrations → Dev

  
type:security

Issue de sécurité → Medic ou Dev

No labels
autopilot:pending-human
source:analyste
source:defenseur
source:human
source:medic
status:approved
status:blocked
status:in-progress
status:needs-clarification
status:needs-fix
status:ready
status:review
status:triage
type:bug
type:feature
type:infra
type:refactor
type:schema
type:security
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: maximus/vps-health-api#46
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?