Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Modifier les tests qui utilisent le réseau #2287

Closed
1 of 2 tasks
fchabouis opened this issue Mar 31, 2022 · 6 comments · Fixed by #2295
Closed
1 of 2 tasks

Modifier les tests qui utilisent le réseau #2287

fchabouis opened this issue Mar 31, 2022 · 6 comments · Fixed by #2295
Labels
dette technique Entretien & maintenance générale, nécessaire pour que le code reste de bonne qualité

Comments

@fchabouis
Copy link
Contributor

fchabouis commented Mar 31, 2022 •
edited
Loading

J'ai remarqué qu'en lançant certains tests, on utilise le réseau. Ce n'est probablement pas voulu.

image

Je vais lister ici au fur et à mesure les fichiers concernés :
@fchabouis fchabouis added the dette technique Entretien & maintenance générale, nécessaire pour que le code reste de bonne qualité label Mar 31, 2022
@thbar
Copy link
Contributor

thbar commented Mar 31, 2022

@fchabouis pour info en lien relatif, je suis en train de gérer celui-ci: #1751 ; j'édite la description pour l'ajouter

thbar added a commit that referenced this issue Mar 31, 2022
@thbar
Stop using network during tests (#1751, #2287)
948b8c7 
This creates a failing test, more refactoring will come.
@thbar thbar mentioned this issue Mar 31, 2022
Merged
7 tasks
@fchabouis fchabouis mentioned this issue Apr 4, 2022
Merged
thbar added a commit that referenced this issue Apr 26, 2022
@thbar @AntoineAugusti
Mise sous tests et nettoyage de printemps des envois d'email (#2290)
10ada52 
* Stop using network during tests (#1751, #2287)

This creates a failing test, more refactoring will come.

* Add clean test

* Mix format

* Add wip alternate implementation coverage

* Add test-case for the other implementation

* Setup behaviour (with a slightly different signature on purpose)

The boolean flag is partially a trick to cope with the lack of proper testing setup. I remove it so that it can signal (via compilation errors) where it is used.

* Setup configuration

* Setup mock

* Modify controller to use higher level interface

* Fix test

* Fix test

* Use new behaviour

I'm filtering when is_blank is true

* Add moduledoc

* Add todos

* Replace implementation by wrapper

* Mix format

* Replace mock by mox (now that the behaviour is in place)

* Email should not be sent

* Remove legacy code used for testing or skipping send

* Mark impl

* Remove now obsolete test

* Remove unused import

* Add systematic verify (to fix a test checking this but also make sure it happens)

* Remove all the "blanks"

After discussing with Francis & Antoine, it seems that it is more a testing technique than anything else. I am moving to raw calls, then will adapt tests & relevant APIs, to ensure we can have more transparent code.

* Adapt test API (no more blank) and add TODO

* Adapt test API (no more blank) and add TODO

* Adapt test API (no more blank) and add TODO

* Run "mix format"

* Bring back meaningful test

* Bring back another meaningful test

* Bring back yet another test

* Remove import

* Fix credo warning

* Remove todo for now (I will add a PR comment that this isn't tested yet)

* Remove dead code

* Remove dead code (already muted in #1189)

* Add todos

* Add sample config file to make it easy to work locally

* Add a dummy implementation for dev

* Remove unused alias

* Clarify confusing (to me) code

* Make dev dummy email work

* Add note

* Replace HTTPoison call by wrapper for testability

* Implement test for obscure inactive_data code

* Remove TODO

* Add doc

* outdated_data should not send empty email

* Format code

* Document inactive_data in full (doc + tests)

* Add missing test for outdated_data overall behaviour

* Restructure test into describe block

* Rename file to match module name

* Try to trigger CircleCI

* Trigger build

* Add verify!

* Update apps/transport/test/transport/mailjet_client_test.exs

Co-authored-by: Antoine Augusti <[email protected]>

* Rename topic to subject

Co-authored-by: Antoine Augusti <[email protected]>
@thbar
Copy link
Contributor

thbar commented Sep 29, 2022

Il en reste, je réouvre et je vais donner des éléments (découvert en travaillant sur #1853).

@thbar thbar reopened this Sep 29, 2022
@thbar
Copy link
Contributor

thbar commented Sep 29, 2022

Les joyeux coupables sont tous les tests qui font une de ces invocations :

Mox.stub_with(Transport.HTTPoison.Mock, HTTPoison)
Mox.stub_with(Transport.AvailabilityChecker.Mock, Transport.AvailabilityChecker)

Ca cela fait que le mock invoque du vrai code qui requête.

@AntoineAugusti
Copy link
Member

@etalab/transport-tech c'est résolu ça non ?

@thbar
Copy link
Contributor

thbar commented Jul 11, 2023

@AntoineAugusti je crois que oui, on pourra réouvrir au besoin.

@thbar thbar closed this as completed Jul 11, 2023
@thbar
Copy link
Contributor

thbar commented Jul 11, 2023

Les appels que j'avais mentionné plus haut sont toujours potentiellement problématiques, mais je pense qu'on doit faire autre chose à côté qui fait que ça ne dérange pas. On verra si ça se représente !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
dette technique Entretien & maintenance générale, nécessaire pour que le code reste de bonne qualité
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Tests : pas d'appels réseaux ! etalab/transport-site
3 participants
@thbar @AntoineAugusti @fchabouis