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.

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

Add a kernelReset attribute to controle kernel reseting on client creation. #56

Merged
merged 2 commits into from Jan 28, 2014
Merged

Conversation

@SylvG
Copy link

@SylvG SylvG commented Dec 5, 2013

During tests of controllers with a database connection inside, the connection is lost by kernel reseting.
This PR allows to disable kernel reseting and avoid this problem.

Note : I don't know how to test it.

@SylvG
Copy link
Author

@SylvG SylvG commented Dec 6, 2013

En fait quand on utilise une bdd chargée en mémoire (SQLite en l'occurrence) et qu'on la charge dans le beforeTestMethod() pour un test de contrôleur, la connexion est resetée par le createClient() qui fait un reboot du kernel.

Du coup ce booléen permet de shunter ce reset.

@stephpy
Copy link
Member

@stephpy stephpy commented Dec 6, 2013

En effet, c'est un bon exemple. 👍

@jubianchi
Copy link
Member

@jubianchi jubianchi commented Dec 6, 2013

👍 j'en avais discuté un moment avec @mikaelrandy : c'est effectivement quelque chose de très utile pour tester ses controllers.

Par contre, j'aurais activé les annotations pour activer/désactiver le reset du kernel : ça permettrait d'avoir un comportement semblable à ce qui est fait dans atoum pour modifier le comportement par défaut des tests.

@mikaelrandy
Copy link
Member

@mikaelrandy mikaelrandy commented Dec 8, 2013

@jubianchi @SylvG travaille avec moi, et c'est le résultat de notre discussion cette PR ;)
Tu peux nous pointer un exemple d'implémentation de ce que tu attends pour qu'on adapte notre PR ?

@jubianchi
Copy link
Member

@jubianchi jubianchi commented Dec 8, 2013

@mikaelrandy @SylvG je m'en doutais ;)

Concernant les annotations, voici quelques exemples de patch appliqué sur atoum. Je pense que vous pourrez vous en inspirer pour ajouter le support des annotations ici :

@mikaelrandy
Copy link
Member

@mikaelrandy mikaelrandy commented Jan 3, 2014

Je viens d'ajouter le support des annotations pour cette PR.

A noter que j'ai fais le choix de la surcharge de la méthode plutôt que du préchargement de l'annotation extractor, car ce dernier est systèmatiquement reseté, et donc que les annotation pré-chargées sont supprimées.

@jubianchi
Copy link
Member

@jubianchi jubianchi commented Jan 3, 2014

ça me semble correct, je vais essayer de monter un petit test ce week-end ;)

merci 👍

@KuiKui
Copy link

@KuiKui KuiKui commented Jan 15, 2014

up 😄

@jubianchi
Copy link
Member

@jubianchi jubianchi commented Jan 16, 2014

@KuiKui @mikaelrandy tant que les tests ne sont pas verts, rien ne sera mergé... je dois regardé ça en détail parce que l'erreur me parait bizarre dans le contexte de cette PR.

@omansour
Copy link

@omansour omansour commented Jan 23, 2014

@jubianchi jubianchi merged commit 744db40 into atoum:master Jan 28, 2014
1 check failed
1 check failed
default The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants