-
Notifications
You must be signed in to change notification settings - Fork 57
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
[FEATURE] Créé des scripts pour générer des élements Modulix (PIX-11270) #8097
[FEATURE] Créé des scripts pour générer des élements Modulix (PIX-11270) #8097
Conversation
Une fois les applications déployées, elles seront accessibles via les liens suivants : Les variables d'environnement seront accessibles via les liens suivants : |
8d81473
to
225b5a4
Compare
34e8c0b
to
9b5702a
Compare
@@ -1,5 +1,3 @@ | |||
#!/usr/bin/env node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Est-ce que ce commit est nécessaire ? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je trouvais intéressant car à la base je trouvais l'UX meilleure mais le linter m'a embêté. Quand on aura monté de version de "n" on aura peut-être moyen de faire mieux en revertant ce commit.
const result = imageElementSchema.validate(getImageSample()); | ||
|
||
// Then | ||
expect(result.error).to.equal(undefined, result.error?.details.map((error) => error.message).join('. ')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
En regardant de plus près avec @mariannebost et @mirawlin, nous voyons pas vraiment l'intérêt de nommer l'erreur :
Voici les erreurs pour chaque expect :
expect(result.error).to.equal(undefined, result.error?.details.map((error) => error.message).join('. '));
// AssertionError: "alt" is required: expected ValidationError: "alt" is required { …(2) } to equal undefined
expect(result.error).to.equal(undefined);
// AssertionError: expected ValidationError: "alt" is required { …(2) } to equal undefined
expect(result.error).to.be.undefined;
// AssertionError: expected ValidationError: "alt" is required { …(2) } to be undefined
Comme cela complexifie vachement la lecture des expects : il faut savoir que le deuxième paramètre est un override de l'erreur, et que result.error?.details.map((error) => error.message).join('. ')
a pour but d'afficher toutes les erreurs du schéma. Nous proposons plutôt de rester simple avec :
expect(result.error).to.equal(undefined, result.error?.details.map((error) => error.message).join('. ')); | |
expect(result.error).to.be.undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pour info c'est un copié collé de ce qui est fait côté module-datasource_test.
Chaud pour voir ensemble comment faire mieux !
9b5702a
to
59931fc
Compare
🦄 Problème
La contribution aux Modulix est un peu limitée pour le moment :
🤖 Proposition
Donner accès à des scripts dans l'API pour générer des éléments :
Chacun de ces scripts renvoie l'élément demandé au format à jour avec un nouvel UUID ✨ .
Il est aussi possible de copier directement en ajoutant
| pbcopy
(sur Mac). Une ligne complète donne donc :node ./api/scripts/modulix/get-sample-qcm-element.js | pbcopy
🌈 Remarques
J'avais prévu initialement de permettre l'usage de script directement en bash grâce à l'usage du shebang mais notre linter nous en empêche. À voir une fois que nous seront passé dans la version 17 de
n
si l'optionignoreUnpublished
nous permet de mettre ça en place.💯 Pour tester
Récupérer la branche en local et vérifier que chaque script génère bien le bon type d'élément !