Skip to content

Update regress tests#444

Open
ACornuIGN wants to merge 13 commits intoign-packo:2Opifrom
ACornuIGN:fix_unit_test
Open

Update regress tests#444
ACornuIGN wants to merge 13 commits intoign-packo:2Opifrom
ACornuIGN:fix_unit_test

Conversation

@ACornuIGN
Copy link
Copy Markdown
Collaborator

@ACornuIGN ACornuIGN commented Mar 5, 2026

Correction d'un ancien nom de balise id_type qui est devenu is_auto

Modification du nom et de la structure des balises des geojson de saisie et pour les tests unitaires

@ACornuIGN
Copy link
Copy Markdown
Collaborator Author

ACornuIGN commented Mar 5, 2026

PR en WIP, pour correction des tests vector

@ACornuIGN ACornuIGN force-pushed the fix_unit_test branch 2 times, most recently from 0321fd0 to 796f011 Compare March 6, 2026 15:19
@ACornuIGN
Copy link
Copy Markdown
Collaborator Author

PR prête a être revu

@ftoromanoff ftoromanoff self-requested a review March 11, 2026 09:59
Copy link
Copy Markdown
Collaborator

@ftoromanoff ftoromanoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo dans le 2eme nom de commit

Comment thread db/db.js Outdated
Comment thread middlewares/branch.js
Comment thread middlewares/patch.js Outdated
Comment thread routes/patch.js
opi1: 'opiRef',
opi2: 'opiSec',
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je trouvais ca quand même intéressant d'avoir une possibilité de factorisation et généralisation de code..

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je peux le remettre mais il ressemblera à ça:

const alias = {
  opi1: 'opiName',
  opi2: 'opiNameSec',
  color1: 'color',
  color2: 'colorSec',
}

Est-ce que cela vaut vraiment le coût ? C'est une surcharge de code dans ce cas non (sert à rien).
L'alias fonctionnait bien dans le cas ou on avec une balise opi1 qui regroupait toutes les données de opi1 et idem pour opi2, mais ce n'est pas le cas ici.

Comment thread routes/patch.js Outdated
pgClient.close,
returnMsg);

router.get('/:idBranch/lastpatches',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toutes ses lignes de code sont les mêmes que juste au dessus, il faudrait les factoriser, de telle manière que si on change l'un on est pas besoin de changer l'autre...

Il ne manquerait pas le nombre de patchs demandés d'ailleurs ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Désolé un récupération de commit mal fait, n'a rien à faire ici, je le supprime

Comment thread regress/test-API/3_branch.js Outdated
patchIsAuto: false,
color: overviews.list_OPI[testOpi].color,
opiName: testOpi,
auto: false,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seulement le cas auto: false est testé de ce que je vois ?
Il manque la moitié des tests alors

@ACornuIGN ACornuIGN changed the title Upgrading unit tests Changing the names of input tags and upgrading unit tests Mar 11, 2026
@ACornuIGN ACornuIGN requested a review from ftoromanoff March 16, 2026 09:15
Copy link
Copy Markdown
Collaborator

@ftoromanoff ftoromanoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

le commit est toujours a renommer.. Et des commits en Anglais d'autre en Français...

Comment thread itowns/Editing.js
}),
opiNameSec: this.opi2Name,
colorSec: this.opi2Color}
),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pour moi, c'est une regression

Comment thread db/db.js
Comment thread middlewares/patch.js Outdated
feature.properties.opiName,
patchIsAuto ? feature.properties.colorSec : null,
patchIsAuto ? feature.properties.opiNameSec : null,
opi1.with_rgb,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alors pourquoi Opi1 et opi2 si vous avez choisi de garder opi et opiSec ???

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un oublie à modifier

Comment thread regress/test-API/4_patch.js
@ACornuIGN
Copy link
Copy Markdown
Collaborator Author

rebase sur 2Opi effectué

@ACornuIGN ACornuIGN requested a review from ftoromanoff April 16, 2026 07:00
Comment thread regress/test-API/4_patch.js Outdated
done();
});
}).timeout(9000);
it("should get an error: 'File(s) missing / out of boundaries", (done) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plutôt que le supprimer, ne faudrait il pas le mettre a skip ?
Si tu le supprimer ca veut dire que ce tests ne sert a rien : il ne test rien dans le code, alors que rajouter skip voudrait plutot dire, que la partie de code associée pourrait être tester mais ne l'est pas (causes diverses)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ici le test est un doublon du test de polygone "should get an error: 'File(s) missing / out of boundaries", donc on peut le supprimer

});

describe('PUT /{idBranch}/patch/undo', () => {
it("should return 'undo: patch 2 annulé'", (done) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un nouveau test qui n'est pas forcement utile, vu que cette partie de code est déjà tester dans le test suivant.
Probablement aussi lié à une confusion entre test unitaire et fonctionnel. De ce que j'ai compris on ne fais pas vraiment des tests unitaires (qui testerait chaque blocs fonctionnels de code) mais plutôt l'entièreté d'une route ?

Comment thread regress/test-API/4_patch.js Outdated

describe('PUT /{idBranch}/patch/redo', () => {
it("should return 'redo: patch xxx réappliqué'", (done) => {
it("should return 'redo: patch xxx Polygone réappliqué'", (done) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Y a t'il vraiment une différence fonctionnelle entre réappliquer un patch Polygone et un patch MultiLinestring ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non il n'y a pas de différence comme pour le undo, il n'y a pas de différence non plus

@ACornuIGN ACornuIGN self-assigned this Apr 16, 2026
@ACornuIGN ACornuIGN changed the title Changing the names of input tags and upgrading unit tests Update tests Apr 21, 2026
@ACornuIGN ACornuIGN changed the title Update tests Update regress tests Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants