Le kata Digging Estimator – Part 2 – Approval testing

Cet article est le deuxième de la série concernant le kata Digging Estimator, un kata créé par mes soins à destinations des élèves d’un module de méthodologie de tests.

Dans cette partie, nous allons tenter de comprendre le comportement du code à l’aide de l’approval testing.

Les autres articles :

Vous pouvez trouver le code terminé, avec le détail des commits sur la branche ts-solution du github

Une bonne et une mauvaise nouvelle

Après une installation des dépendances sans soucis (npm install), on s’aperçoit avec joie qu’il existe déjà un test !

describe("digging estimator", () => {

  it("should return as Dr Pockovsky said", () => {
    // To have it work, you need to go set the rates to [0, 3, 5.5, 7]
    const estimator = new DiggingEstimator();

    const result = estimator.tunnel(28, 2, "granite");

    expect(result.total).toBe(48);
  });
});

Comme l’indique subtilement le commentaire, cependant, le test ne marche pas :

npm test

Error: Does not work in test mode

Une rapide recherche sur l’erreur donne :

private get(rockType: string) : number[] {
    ...
    const url = `dtp://research.vin.co/digging-rate/${rockType}`;
    console.log(`Tried to fetch ${url}`);
    throw new Error('Does not work in test mode');
  }

Cette méthode get est appelée d’entrée de jeu par notre fonction tunnel :

export class DiggingEstimator {
  tunnel(length: number, days: number, rockType: string): TeamComposition {
    const digPerRotation = this.get(rockType);

Concrètement, mon DiggingEstimator dépend d’une méthode qui va chercher des éléments à l’extérieur. Cet appel ne fait pas partie de mon unité de test, j’ai besoin de la bouchonner. Sauf que la méthode est privée.

Face à une dépendance de ce type, j’ai trois solutions :

je passe la méthode en visibilité public et j’utilise jest.mock pour la remplacer.
je passe la méthode en visibilité protected, et je peux alors créer une surcharge de DiggingEstimator et faire un override de get
j’injecte une interface pour aller faire les appels à l’extérieur

Sandro Mancuso illustre bien le problème dans sa vidéo d’implémentation du kata Trip Service. Nous n’avons pas encore de tests, donc nous voulons minimiser les modifications sur le code de production, mais on ne peut pas tester sans cette modification. La méthode 1 est la plus simple, mais elle ne reflète ni mon intention, ni le SRP. Dans mon cas, j’estime qu’extraire le code derrière une interface me parait assez trivial, donc je me lance. Je crée mon interface :

export interface RockInformationInterface {
  get(rockType: string): number[];
}

Puis je crée le service correspondant implémenté pour la Vigilante Institution Naine :

import { RockInformationInterface } from "../rock-information.interface";

export class VinRockInformationService implements RockInformationInterface {
  get(rockType: string) : number[] {
    // ...
    const url = `dtp://research.vin.co/digging-rate/${rockType}`;
    console.log(`Tried to fetch ${url}`);
    throw new Error('Does not work in test mode');
  }
}

Il ne me reste plus qu’à injecter ce service dans le code :

export class DiggingEstimator {

  constructor(private readonly rockInformation: RockInformationInterface) {}

  tunnel(length: number, days: number, rockType: string): TeamComposition {
    const digPerRotation = this.rockInformation.get(rockType);

J’ai cependant un problème : les tests cassent, car le constructeur de DiggingEstimator n’attendait pas de paramètre à l’origine. J’ai cassé la rétro-compatibilité : tous les clients qui utilisent déjà mon DiggingEstimator ne fonctionnent plus, et cela n’est pas acceptable. La solution, c’est de rendre ce paramètre optionnel et de le remplir par défaut :

export class DiggingEstimator {
  private readonly rockInformation: RockInformationInterface;

  constructor(rockInformation?: RockInformationInterface) {
    this.rockInformation = rockInformation || new VinRockInformationService()
  }

Ok, le code compile à nouveau, mais le test ne passe toujours pas. Pour qu’il passe, je dois lui fournir une autre implémentation de l’interface :

export class FakeRockInformationService implements RockInformationInterface {
  get(): number[] {
    return [0, 3, 5.5, 7];
  }
}

describe("digging estimator", () => {
  it("should return as Dr Pockovsky said", () => {
    const estimator = new DiggingEstimator(new FakeRockInformationService());

    const result = estimator.tunnel(28, 2, "granite");

    expect(result.total).toBe(48);
  });
});

Le test passe, hourra ! Le temps de supprimer la méthode DiggingEstimator.get, qui n’est plus utilisée (mon IDE me le dit), et c’est l’heure de faire un commit : Extract service interface to enable testing

Tester les premiers cas

En approval testing, on cherche à tester les chemins les plus courts en premier. Cela nous permet de comprendre le problème petit à petit, et pas tout à la fois.

Les premières lignes de code sont plutôt claires :

const digPerRotation = this.rockInformation.get(rockType);
const maxDigPerRotation = digPerRotation[digPerRotation.length - 1];
const maxDigPerDay = 2 * maxDigPerRotation;

if (Math.floor(length) !== length || Math.floor(days) !== days || length < 0 || days < 0) {
  throw new InvalidFormatException();
}

Les 3 premières lignes ne sont pas utilisées pour vérifier le cas au-dessus. Je le note dans un coin de ma todo list pour le refactoring. Je suis presque sûr que je pourrais inverser les lignes, mais je n’ai toujours pas mon filet de test. Et comme le disent les trapézistes : « ne travaillez jamais sans filet« .

Bref, pour en revenir à la ligne 5, tout ce qu’on vérifie, ce sont les paramètres en entrée, et on vérifie que ce sont des entiers positifs. Cela semble testable :

it("should check days and length are positive integers", function() {
  const estimator = new DiggingEstimator(new FakeRockInformationService());

  expect(() => estimator.tunnel(-1, 2, "granite")).toThrow(new InvalidFormatException());
  expect(() => estimator.tunnel(2.5, 2, "granite")).toThrow(new InvalidFormatException());
  expect(() => estimator.tunnel(2, -1, "granite")).toThrow(new InvalidFormatException());
  expect(() => estimator.tunnel(2, 5.01, "granite")).toThrow(new InvalidFormatException());
});

On pourrait écrire quatre tests. Ou deux. Dans ce cas précis, je traite un cas fonctionnel global (la gestion du format), je décide de laisser cela comme ça. À un détail près : la ligne 2, qui crée le DiggingEstimator, est déjà présente dans le premier test. Donc je vais refactorer tout ça.

describe("digging estimator", () => {
  let estimator: DiggingEstimator;
  
  beforeEach(() => {
    estimator = new DiggingEstimator(new FakeRockInformationService());
  });

Tout est vert, c’est l’heure pour un petit commit : add parameter validation to approval tests

Les maths commencent

La ligne de code suivante demande un peu plus de réflexion:

const digPerRotation = this.rockInformation.get(rockType);
const maxDigPerRotation = digPerRotation[digPerRotation.length - 1];
const maxDigPerDay = 2 * maxDigPerRotation;
...
if (Math.floor(length / days) > maxDigPerDay) {
  throw new TunnelTooLongForDelayException();
}

On cherche visiblement à vérifier la longueur maximal du tunnel qu’on peut creuser. D’après la spec, on peut avoir au maximum deux rotations de trois miners. Comme j’ai mis dans mon fake qu’on pouvait creuser au maximum sur 7 mètres pour une rotation, ça nous fait 14 mètres par jour. Essayons :

it("should return exception if tunnel is too long to be dug during delay", function() {
  expect(() => estimator.tunnel(29, 2, "granite")).toThrow(new TunnelTooLongForDelayException());
});

C’est un échec :

Received function did not throw

À bien y regarder (et débugger à l’appui), cette fonction Math.floor n’a probablement rien à faire là. En effet, avec 30, ça marche. Je vais remonter ça au responsable fonctionnel.

En approval testing, on ne corrige pas immédiatement les bugs qu’on détecte. Le but est de capturer le comportement actuel du système, pas comment il devrait fonctionner. On corrigera plus tard, si le bug est confirmé.

Rien à voir, mais on va en profiter pour mettre la chaine granite dans une variable. Je n’aime pas les Magic String:

const GRANITE = "granite";
it("should return exception if tunnel is too long to be dug during delay", function() {
    expect(() => estimator.tunnel(30, 2, GRANITE)).toThrow(new TunnelTooLongForDelayException());
  });

Ok, petit commit : add validation of distance to dig

Le comportement nominal

Jusque là, on était dans les exceptions. Il est l’heure de tester les cas qui renvoient une valeur. L’objet renvoyé est une TeamComposition :

export class Team {
  miners = 0;
  healers = 0;
  smithies = 0;
  lighters = 0;
  innKeepers = 0;
  guards = 0;
  guardManagers = 0;
  washers = 0;
}

export class TeamComposition {
  dayTeam: Team = new Team();
  nightTeam: Team = new Team();

  total = 0;
}

À première vue, ça fait un paquet d’infos à vérifier. Allons-y en incrémental :

// Miners
for (let i = 0; i < digPerRotation.length - 1; ++i) {
  if (digPerRotation[i] < Math.floor(length / days)) {
    composition.dayTeam.miners++;
  }
}

OK, on commence par les mineurs. C’est écrit dans le commentaire de toutes façons (note pour plus tard : le commentaire ne sert à rien).

it("should request one dwarf to mine during the day when there is note more than 3 meters to dig", function() {
    const result = estimator.tunnel(5, 2, GRANITE);

    expect(result.dayTeam.miners).toBe(1);
  });

Ok, le test passe. Faisons la même chose pour 2, puis 3 mineurs. En remplaçant le 3 meters par des variables, tant qu’à faire.

Note à l’édition : si vous faites ça, sachez que votre IDE ne retrouvera pas le test si vous cliquez dessus, et ne pourra pas lancer le test en débug.

const ONE_DWARF_DIG_PER_DAY = 3
const TWO_DWARVES_DIG_PER_DAY = 5.5
const THREE_DWARVES_DIG_PER_DAY = 7

export class FakeRockInformationService implements RockInformationInterface {
  get(): number[] {
    return [0, ONE_DWARF_DIG_PER_DAY, TWO_DWARVES_DIG_PER_DAY, THREE_DWARVES_DIG_PER_DAY];
  }
}

it(`should request two day miners for ${TWO_DWARVES_DIG_PER_DAY} to dig per day`, function() {
  const result = estimator.tunnel(Math.floor(TWO_DWARVES_DIG_PER_DAY * 3), 3, GRANITE);

  expect(result.dayTeam.miners).toBe(2);
});

it(`should request three day miners for ${THREE_DWARVES_DIG_PER_DAY} to dig per day`, function() {
  const result = estimator.tunnel(Math.floor(THREE_DWARVES_DIG_PER_DAY * 3), 3, GRANITE);

  expect(result.dayTeam.miners).toBe(3);
});

Petit commit : add day team miners to tests

Reste à faire pareil pour les mineurs de nuit, dont voilà le code :

if (Math.floor(length / days) > maxDigPerRotation) {
  for (let i = 0; i < digPerRotation.length - 1; ++i) {
    if (digPerRotation[i] + maxDigPerRotation < Math.floor(length / days)) {
      composition.nightTeam.miners++;
    }
  }
}

Il n’y a pas de mineur de nuit si on peut tout creuser en utilisant des rotations en journée. Intuitivement, c’est logique : il faut bien plus de personnel de support pour la nuit. Capturons cela dans un test :

it(`should not request any night miner for less than ${THREE_DWARVES_DIG_PER_DAY} to dig per day`, function() {
  const result = estimator.tunnel(Math.floor(THREE_DWARVES_DIG_PER_DAY * 3), 3, GRANITE);

  expect(result.nightTeam.miners).toBe(0);
});

Ok. Vert aussi. Du coup on peut mettre en place les tests des mineurs au-delà.

  it(`should request one night miner for additional ${ONE_DWARF_DIG_PER_DAY} to dig per day`, function() {
    const result = estimator.tunnel(Math.floor(THREE_DWARVES_DIG_PER_DAY + ONE_DWARF_DIG_PER_DAY) , 1, GRANITE);

    expect(result.nightTeam.miners).toBe(0);
  });

Rouge. Expected 0, got 1. Ça m’apprendra à copier-coller. En même temps, ça ne fait pas de mal de vérifier que mes tests échouent de temps en temps, pour être sûr qu’ils font bien quelque chose.

Notez le THREE_DWARVES_DIG_PER_DAY + ONE_DWARF_DIG_PER_DAY. J’aurais sûrement pu le raccourcir, mais j’aime bien le fait d’expliciter. D’ailleurs, ça manque d’explicite en fait. Renommons un coup :

it(`should request one night miner for additional ${ONE_DWARF_DIG_PER_ROTATION} to dig per day`, function() {
  const distanceToDig = Math.floor(THREE_DWARVES_DIG_PER_ROTATION + ONE_DWARF_DIG_PER_ROTATION);

  const result = estimator.tunnel(distanceToDig , 1, GRANITE);

  expect(result.nightTeam.miners).toBe(1);
});

Oui, le test est plus long. Mais il est plus explicite. Et désormais, il passe. Faisons pareil pour les deux autres cas :

 it(`should request two night miners for additional ${TWO_DWARVES_DIG_PER_ROTATION} to dig per day`, function() {
    const distanceToDig = Math.floor(THREE_DWARVES_DIG_PER_ROTATION + TWO_DWARVES_DIG_PER_ROTATION);

    const result = estimator.tunnel(distanceToDig , 1, GRANITE);

    expect(result.nightTeam.miners).toBe(2);
  });

  it(`should request three night miners for additional ${THREE_DWARVES_DIG_PER_ROTATION} to dig per day`, function() {
    const distanceToDig = Math.floor(THREE_DWARVES_DIG_PER_ROTATION + THREE_DWARVES_DIG_PER_ROTATION);

    const result = estimator.tunnel(distanceToDig , 1, GRANITE);

    expect(result.nightTeam.miners).toBe(3);
  });

Ok. Petit commit : add night team miners to tests

Et vint la lumière

Regardons la suite du code :

const dt = composition.dayTeam;
const nt = composition.nightTeam;

if (dt.miners > 0) {
  ++dt.healers;
  ++dt.smithies;
  ++dt.smithies;
}

Passons la syntaxe alambiquée. Le calcul des soigneurs (healers) et des forgerons (smithies) ne dépend que des mineurs. Allons voir le reste du code qui change la dayTeam :

 if (dt.miners > 0) {
   dt.innKeepers = Math.ceil((dt.miners + dt.healers + dt.smithies) / 4) * 4;
   dt.washers = Math.ceil((dt.miners + dt.healers + dt.smithies + dt.innKeepers) / 10);
 }

Intéressant, ça ne dépend que des variables précédentes. Et comme le nombre de miners a peu de possibilités, et que tout dépend d’eux, on peut énumérer le résultat plutôt que de suivre le code. Surtout que c’est la même histoire pour l’équipe de nuit : elle ne dépend que du nombre de mineurs. Alors, prenons un papier et un crayon, prenons la spec, et regardons ça :

	Jour, 1 mineur	Jour, 2 mineurs	Jour, 3 mineurs	Nuit, 1 mineur	Nuit, 2 mineurs	Nuit, 3 mineurs
Mineurs	1	2	3	1	2	3
Soigneurs	1	1	1	1	1	1
Forgerons
Eclaireurs
Taverniers
Gardes
Chefs des gardes
Vaisselle
Total

Un calcul un peu fastidieux

Vous savez quoi ? On va demander au code.

En approval testing, on peut lire le code pour s’assurer de ce qu’il fait, pour savoir quels tests à écrire. On peut aussi lire la spéc pour en déduire les tests (si on a une spec). Ou alors on peut demander au code quel est son comportement. Parce que peu importe l’opinion qu’on en a, c’est le code qui gagne.

Reprenons le test sur 1 mineur de jour et transformons le un petit peu (c’est la ligne 7 qui nous intéresse) :

it(`should request one day miner when there is note more than 3 meters to dig`, function() {
  const result = estimator.tunnel(ONE_DWARF_DIG_PER_ROTATION * 2, 2, GRANITE);

  const expectedDayTeam = new Team();
  expectedDayTeam.miners = 1;

  expect(result.dayTeam).toEqual(expectedDayTeam);
});

C’est rouge. Quelle surprise. Mais ce qui m’intéresse, c’est le message d’erreur :

- Expected  - 4
+ Received  + 4

  Team {
    "guardManagers": 0,
    "guards": 0,
-   "healers": 0,
-   "innKeepers": 0,
+   "healers": 1,
+   "innKeepers": 4,
    "lighters": 0,
    "miners": 1,
-   "smithies": 0,
-   "washers": 0,
+   "smithies": 2,
+   "washers": 1,
  }

Ce n’est pas très naturel à lire, mais on attendait 1 healer, 4 innKeepers, 2 smithies, 1 washers, et 0 du reste. Mettons le test à jour.

it(`should request one day miner when there is note more than 3 meters to dig`, function() {
  const result = estimator.tunnel(ONE_DWARF_DIG_PER_ROTATION * 2, 2, GRANITE);

  const expectedDayTeam = new Team();
  expectedDayTeam.miners = 1;
  expectedDayTeam.healers = 1;
  expectedDayTeam.innKeepers = 4;
  expectedDayTeam.smithies = 2;
  expectedDayTeam.washers = 1;

  expect(result.dayTeam).toEqual(expectedDayTeam);
});

Ok. Ça passe. Mais ce n’est pas très beau, et vu qu’on va le faire pour tous les cas… On va se créer une méthode qui génère la team pour nous.

function createTeam(miners = 0, healers = 0, smithies = 0, lighters = 0, innKeepers = 0, guards = 0, guardManagers = 0, washers = 0): Team {
  const team = new Team();
  team.miners = miners;
  team.healers = healers;
  team.smithies = smithies;
  team.lighters = lighters;
  team.innKeepers = innKeepers;
  team.guards = guards;
  team.guardManagers = guardManagers;
  team.washers = washers;

  return team;
}

it(`should request one day miner when there is note more than 3 meters to dig`, function() {
  const result = estimator.tunnel(ONE_DWARF_DIG_PER_ROTATION * 2, 2, GRANITE);

  const expectedDayTeam = createTeam(1, 1, 2, 0, 4, 0, 0, 1);

  expect(result.dayTeam).toEqual(expectedDayTeam);
});

Ok. Parfait. Je peux oublier la majesté de la fonction createTeam dans un coin et me focaliser sur mes tests. Pour l’équipe de jour :

it(`should request two day miners + according team for ${TWO_DWARVES_DIG_PER_ROTATION} to dig per day`, function() {
  const result = estimator.tunnel(Math.floor(TWO_DWARVES_DIG_PER_ROTATION * 3), 3, GRANITE);

  const expectedDayTeam = createTeam(2, 1, 2, 0, 8, 0, 0, 2);

  expect(result.dayTeam).toEqual(expectedDayTeam);
});

it(`should request three day miners + according team for ${THREE_DWARVES_DIG_PER_ROTATION} to dig per day`, function() {
  const result = estimator.tunnel(Math.floor(THREE_DWARVES_DIG_PER_ROTATION * 3), 3, GRANITE);

  const expectedDayTeam = createTeam(3, 1, 2, 0, 8, 0, 0, 2);

  expect(result.dayTeam).toEqual(expectedDayTeam);
});

Et pour l’équipe de nuit :

it(`should not request any night miner for less than ${THREE_DWARVES_DIG_PER_ROTATION} to dig per day`, function() {
    const result = estimator.tunnel(Math.floor(THREE_DWARVES_DIG_PER_ROTATION * 3), 3, GRANITE);

    const expectedNightTeam = createTeam(0, 0, 0, 0, 0, 0, 0, 0);

    expect(result.nightTeam).toEqual(expectedNightTeam);
  });

  it(`should request one night miner for additional ${ONE_DWARF_DIG_PER_ROTATION} to dig per day`, function() {
    const distanceToDig = Math.floor(THREE_DWARVES_DIG_PER_ROTATION + ONE_DWARF_DIG_PER_ROTATION);

    const result = estimator.tunnel(distanceToDig, 1, GRANITE);

    const expectedNightTeam = createTeam(1, 1, 2, 2, 8, 3, 1, 2);

    expect(result.dayTeam).toEqual(MAX_DAY_TEAM)
    expect(result.nightTeam).toEqual(expectedNightTeam);
  });

  it(`should request two night miners for additional ${TWO_DWARVES_DIG_PER_ROTATION} to dig per day`, function() {
    const distanceToDig = Math.floor(THREE_DWARVES_DIG_PER_ROTATION + TWO_DWARVES_DIG_PER_ROTATION);

    const result = estimator.tunnel(distanceToDig, 1, GRANITE);

    const expectedNightTeam = createTeam(2, 1, 2, 3, 8, 4, 2, 3);

    expect(result.dayTeam).toEqual(MAX_DAY_TEAM)
    expect(result.nightTeam).toEqual(expectedNightTeam);
  });

  it(`should request three night miners for additional ${THREE_DWARVES_DIG_PER_ROTATION} to dig per day`, function() {
    const distanceToDig = Math.floor(THREE_DWARVES_DIG_PER_ROTATION + THREE_DWARVES_DIG_PER_ROTATION);

    const result = estimator.tunnel(distanceToDig, 1, GRANITE);

    const expectedNightTeam = createTeam(3, 1, 2, 4, 12, 5, 2, 3);

    expect(result.dayTeam).toEqual(MAX_DAY_TEAM)
    expect(result.nightTeam).toEqual(expectedNightTeam);
  });

Vous remarquerez que je vérifie l’équipe de jour aussi. J’aurais pu me contenter de le faire sur un test. C’est sûrement un peu trop défensif, mais étonnamment je me sens plus confort avec. Si ça m’embête ensuite, je l’enlèverai.

Petit commit ? check team composition

Il me manque le total. Je le sais, parce qu’en lisant le code j’ai vu cette horreur :

composition.total = dt.miners + dt.washers + dt.healers + dt.smithies + dt.innKeepers +
      nt.miners + nt.washers +  nt.healers  + nt.smithies  + nt.innKeepers + nt.guards + nt.guardManagers + nt.lighters;

Si seulement Team était un objet qui savait additionner… Je note dans mes refactos.

En attendant, on va faire comme tout à l’heure:

it(`should request one day miner + according team when there is note more than 3 meters to dig`, function() {
  const result = estimator.tunnel(ONE_DWARF_DIG_PER_ROTATION * 2, 2, GRANITE);

  const expectedDayTeam = createTeam(1, 1, 2, 0, 4, 0, 0, 1);

  expect(result.dayTeam).toEqual(expectedDayTeam);
  expect(result.total).toBe(-118);
});

Rouge :

Expected: -118
Received: 9

Évidemment. C’était volontaire. Je rectifie le code. J’en profite pour vérifier que 1 + 1 + 2 + 4 + 1 = 9 par acquis de conscience. La calculette me dit que c’est bon. Pas de bug à remonter.

Je fais pareil pour tout le monde, ce qui donne :

	Jour, 1 mineur	Jour, 2 mineurs	Jour, 3 mineurs	Nuit, 1 mineur	Nuit, 2 mineurs	Nuit, 3 mineurs
Mineurs	1	2	3	1	2	3
Soigneurs	1	1	1	1	1	1
Forgerons	2	2	2	2	2	2
Eclaireurs	0	0	0	2	3	4
Taverniers	4	8	8	8	8	12
Gardes	0	0	0	3	4	5
Chefs des gardes	0	0	0	1	2	2
Vaisselle	1	2	2	2	3	3
Total	9	15	16	20	25	32

Un calcul un peu fastidieux

Je mets à jour les tests (en pensant bien à additionner à ma colonne de nuit mes 16 nains de la rotation de jour). Et je me rends compte que je commence à avoir beaucoup d’assertions, et surtout qu’elles sont évaluées séquentiellement :

expect(result.dayTeam).toEqual(MAX_DAY_TEAM)
expect(result.nightTeam).toEqual(expectedNightTeam);
expect(result.total).toBe(48);

Deux options, soit je casse les tests, soit je m’arrange pour que tout soit visible du premier coup. Je vais partir sur la première option. Comme ça si j’ai plusieurs tests qui passent en rouge, je pourrais deviner rien qu’en regardant les noms à quel endroit j’ai cassé quelque chose :

describe(`when there is up to additional ${THREE_DWARVES_DIG_PER_ROTATION} to dig per day`, function() {
    let result: TeamComposition;
    beforeEach(() => {
      const distanceToDig = Math.floor(THREE_DWARVES_DIG_PER_ROTATION + THREE_DWARVES_DIG_PER_ROTATION);

      result = estimator.tunnel(distanceToDig, 1, GRANITE);
    });

    it("should have max day team", function() {
      expect(result.dayTeam).toEqual(MAX_DAY_TEAM)
    });

    it("should have a night team with three miners", function() {
      const expectedNightTeam = createTeam(3, 1, 2, 4, 12, 5, 2, 3);
      expect(result.nightTeam).toEqual(expectedNightTeam);
    });

    it("should have the correct total", function() {
      expect(result.total).toBe(48);
    });
  });

Je fais pareil pour tous les cas, puis commit : add total and refactor for clearer view if I break something

Dernières vérifications

Il me reste à vérifier mon coverage pour être sûr de ne pas avoir raté trop de choses :

npm test

97.29% de couverture de branche sur le fichier digging-estimator.ts.

Pour rappel, la couverture de branche est toujours la plus fiable et la plus précise. Les couvertures de lignes, de fonction et de classe sont très superficielles. D’autre part, la couverture de branche ne me permet pas de m’assurer que mon code est bien testé : il me permet seulement de détecter les zones qui ne sont pas couvertes.

Il me manque un bout de la la ligne 32 :

constructor(rockInformation?: RockInformationInterface) {
   this.rockInformation = rockInformation || new VinRockInformationService()
}

Effectivement, ça mériterait un petit test avec la configuration par défaut.

describe("when not using the fake implementation for the RockInformationService", function() {
  it("should fail", function() {
    const realEstimator = new DiggingEstimator();
    expect(() => realEstimator.tunnel(2, 1, GRANITE)).toThrow();
  });
});

Il est possible que dans un cas réel on ne veuille pas écrire ce test parce qu’il balance une requête sur une API externe, mais là, c’est sans risque.

Et me voilà à 100% de couverture sur mon fichier :

On en profite pour faire un petit test de mutation :

npm run test:mutation

Les tests de mutation vont un cran plus loin que la couverture de branche. Ils effectuent des modifications aléatoires de mon code et vérifient si les tests passent toujours. Si les tests ne deviennent pas rouge, cela veut dire qu’ils ne sont pas très fiables (i.e. qu’ils passent par le code, mais qu’ils ne vérifient pas le comportement). Les tests de mutation ne donnent pas une garantie de qualité absolue, mais ils rassurent encore un peu plus.

94%, c’est plutôt pas mal. En regardant le rapport, je m’aperçois qu’il me manque les cas où je fournis length et day à 0. Le reste des cas semble être des morceaux de code qui ne sont pas pertinents.

Par acquis de conscience, j’ajoute les cas aux limites. Aucune idée de ce qu’ils doivent faire, on va voir. Pas de soucis pour coder la longueur de tunnel de 0, par contre pour la durée, j’ai dû m’y reprendre à plusieurs fois :

it("should return empty teams if tunnel has no length", function() {
  const result = estimator.tunnel(0, 2, GRANITE);
  const emptyTeam = createTeam();

  expect(result.dayTeam).toEqual(emptyTeam);
  expect(result.nightTeam).toEqual(emptyTeam);
  expect(result.total).toBe(0);
});

it("should return empty team if duration is 0", function() {
  expect(() => estimator.tunnel(10, 0, GRANITE)).toThrow(new Error());
});

Pourquoi une exception ? À cause d’une division par 0 dans le code. Il faudra penser à demander au responsable fonctionnel si c’est normal.

On va se contenter de ce qu’on a là. 95% au dernier score de mutation, c’est vraiment pas mal. Dernier commit d’approval : add edge cases with 0 length and duration.

C’est l’heure de passer au refacto !

Conclusion

L’approval testing m’a permis d’explorer mon code petit à petit et de mettre en place le filet de test qui va me permettre de faire mon refacto. La méthode est souvent purement incrémentale, à la lecture du code, mais dans ce cas précis, à l’aide de la spécification, j’ai pu traiter sous un autre angle en énumérant le comportement.

Mes outils de couverture (de branches) et de tests de mutations m’ont permis de m’assurer que mes tests couvraient la majorité des fonctionnalités et qu’elles testaient bien les résultats. Je me sens donc confiant pour passer à la phase 2 : le refacto.

Une bonne et une mauvaise nouvelle

Tester les premiers cas

Les maths commencent

Le comportement nominal

Et vint la lumière

Dernières vérifications

Conclusion

Vous devriez également aimer

Le kata Digging Estimator – Part 4 – Ajout de fonctionnalité

Le kata Digging Estimator – Part 3 – Refactoring

Le kata Digging Estimator – Part 1 – Le problème

Laisser un commentaire Annuler la réponse