Projet

Général

Profil

Development #48464

Le build fait des erreurs s'il est lancé un peu après minuit

Ajouté par Emmanuel Cazenave il y a plus de 3 ans. Mis à jour il y a plus de 3 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
11 novembre 2020
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

https://jenkins.entrouvert.org/job/chrono/934/

À analyser si c'est juste des tests mal écrits ou s'il y a de vrais pépins dans cette tranche horaire.


Fichiers

Révisions associées

Révision 53f7945e (diff)
Ajouté par Valentin Deniaud il y a plus de 3 ans

tests: use current timezone rather than UTC (#48464)

Révision a51a036c (diff)
Ajouté par Valentin Deniaud il y a plus de 3 ans

api: use current timezone in get_max_datetime (#48464)

Historique

#1

Mis à jour par Valentin Deniaud il y a plus de 3 ans

  • Assigné à mis à Valentin Deniaud
#2

Mis à jour par Valentin Deniaud il y a plus de 3 ans

Lancé après minuit sur jenkins, donc heure française, donc UTC+1 : pour reproduire il faut figer le temps vers 23h et des poussières en UTC.

Une fois cela fait, quelques tests ont un localtime() qui manquait, c'est corrigé dans 0001.

(deux tests où changer l'heure ne permet pas de reproduire : test_virtual_agenda_day_view et test_resource_day_view, je sèche)

Bon par contre plus intéressant il y avait un vrai bug dans la conversion délai de réservation min/max, aussi un localtime qui manquait mais bien dans le code, c'est 0002.

Encore plus intéressant, il y a d'autres tests qui ont des conditions dépendant de ces délais, avec des fixtures qui testent le cas « passé 23h », pourquoi ils passent ? Parce qu'ils utilisent un mock foireux,

def mock_now(request, monkeypatch):
    def mockreturn():
        return make_aware(request.param)

make_aware par défaut retournant la date avec la timezone courante, alors que now doit toujours renvoyer de l'UTC. Donc puisqu'on moque now() en lui faisant renvoyer localtime(), on ne détecte pas les endroits où il manque localtime.

Et forcément essayer de changer ça en simplement

def mock_now(request, freezer):
    freezer.move_to(request.param)

casse beaucoup de choses, j'ai pas l'impression qu'il suffise de rajouter des localtime() et compagnie pour que les tests cassés passent à nouveau, ça renvoie plutôt à des changements de logique :/ à voir dans un autre ticket donc si on estime que ça vaut le coup de redresser la barre.

#3

Mis à jour par Emmanuel Cazenave il y a plus de 3 ans

Ok pour 0001.

Sur 0002 mes neurones fondent.

#4

Mis à jour par Valentin Deniaud il y a plus de 3 ans

Emmanuel Cazenave a écrit :

Sur 0002 mes neurones fondent.

Il est 00:30, le 12/12, en France. Je veux m'inscrire au créneau piscine le lendemain, 13/12. Il y a un délai qui fait qu'on ne peut s'inscrire que la veille, maximal_booking_delay = 1.
Sauf que sur le serveur, now() dit 23:30 le 11/12. 13 - 11 = 2 > 1, on ne voit pas le créneau alors qu'on devrait.

#5

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Valentin Deniaud a écrit :

Il est 00:30, le 12/12, en France. Je veux m'inscrire au créneau piscine le lendemain, 13/12. Il y a un délai qui fait qu'on ne peut s'inscrire que la veille, maximal_booking_delay = 1.
Sauf que sur le serveur, now() dit 23:30 le 11/12. 13 - 11 = 2 > 1, on ne voit pas le créneau alors qu'on devrait.

Je valide 0002, il manque juste un rebasage au dessus de #48078.

#6

Mis à jour par Valentin Deniaud il y a plus de 3 ans

Benjamin Dauvergne a écrit :

Je valide 0002, il manque juste un rebasage au dessus de #48078.

Done.
En passant, un avis sur le mock_now dont je parle plus haut, le laisser tranquille ou essayer de faire mieux dans un autre ticket ?

#7

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Valentin Deniaud a écrit :

Benjamin Dauvergne a écrit :

Je valide 0002, il manque juste un rebasage au dessus de #48078.

Done.
En passant, un avis sur le mock_now dont je parle plus haut, le laisser tranquille ou essayer de faire mieux dans un autre ticket ?

Personnellement freezer est pour moi plus stable, plus testé et plus compréhensible qu'un simple mock de now() qui laisse beaucoup de choses à désirer, donc à titre personnel je dirai oui. Mais pas dans ce ticket.

#8

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Statut changé de Solution proposée à Solution validée
#9

Mis à jour par Valentin Deniaud il y a plus de 3 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit a51a036c130ff79ee5a0cb2583c948db513c4dff
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Thu Nov 12 15:26:49 2020 +0100

    api: use current timezone in get_max_datetime (#48464)

commit 53f7945e78a4d50df67ee79d01e5f617dcd773aa
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Thu Nov 12 15:05:07 2020 +0100

    tests: use current timezone rather than UTC (#48464)
#10

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF