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

REF Tests #174

Merged
merged 1 commit into from
Oct 3, 2018
Merged

REF Tests #174

merged 1 commit into from
Oct 3, 2018

Conversation

nfaugout-lucca
Copy link
Contributor

@nfaugout-lucca nfaugout-lucca commented Oct 3, 2018

Infra.Tests run under a disposable localdb database, with a GUID name. This ensure that SQL constraints will be respected.

User now has a Guid identity column in order to avoir IDENTITY_INSERT ON/OFF on localdb database (cf dotnet/efcore#703)

I have used a TestFixture to encapsulate the creation/deletion of the database

Domain & Web tests still use InMemory C# storage for speed.

@codecov-io
Copy link

codecov-io commented Oct 3, 2018

Codecov Report

Merging #174 into master will increase coverage by 0.72%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #174      +/-   ##
==========================================
+ Coverage   70.14%   70.87%   +0.72%     
==========================================
  Files         114      114              
  Lines        2820     2822       +2     
  Branches      283      283              
==========================================
+ Hits         1978     2000      +22     
+ Misses        719      698      -21     
- Partials      123      124       +1
Impacted Files Coverage Δ
Domain/RDD.Domain.Mocks/Hierarchy.cs 73.33% <0%> (-6.67%) ⬇️
...D.Web/Helpers/HttpStatusCodeExceptionMiddleware.cs 78.72% <0%> (-1.72%) ⬇️
Infra/RDD.Infra/Storage/InMemoryStorageService.cs 78.08% <0%> (+30.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0eda32d...2e77278. Read the comment docs.

@nfaugout-lucca
Copy link
Contributor Author

Je vois que AppVeyor arrive à faire tourner les tests qui reposent sur localdb, ça fait plaisir !

[Fact]
public async Task PostShouldNotCallGetByIdOnTheCollection()
{
using (var storage = _newStorage(Guid.NewGuid().ToString()))
await _fixture.RunCodeInsideIsolatedDatabaseAsync(async (context) =>
Copy link
Contributor

@rducom rducom Oct 3, 2018

Choose a reason for hiding this comment

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

Si ici on créé une db pour chaque test, alors autant se passer de class fixture et passer par une classe de base dont chaque test hérite (on a aucun état partagé entre les tests) : Xunit garanti que le dispose est appelé pour chaque test, donc RunCodeInsideIsolatedDatabaseAsync deviendrait 1 appel NewDbContext(Guid.NewGuid()) + Database.EnsureCreated(); dans le constructeur, et Database.EnsureDeleted(); + context.Dispose() dans le Dispose de la classe de base.

Ca permettra de rendre le code des tests moins verbeux en retirant tous les appels à RunCodeInsideIsolatedDatabaseAsync

Copy link
Contributor

@Poltuu Poltuu left a comment

Choose a reason for hiding this comment

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

Je suis absolument pour l'option proposée par cette PR, mais contre son utilisation massive sur tous les tests.
Il faut différe
ncier dans les tests ce que l'on teste, je veux dire, parfois on veut vérifier que ça marche bien sur la base SQL d'accord, mais qq fois, on veut vérifier un comportement qqconque sur des objets c# sans préoccupation des requêtes générées.

Egalement les tests aujourd'hui se jouent en moins de 10sec, et cela participe grandement à la qualité du code en général.

Tests run under a disposable localdb database, with a GUID name. This ensure that SQL constraints will be respected.
Tests run in 24 sec now.
User now has a Guid identity column in order to avoir IDENTITY_INSERT ON/OFF on localdb database (cf dotnet/efcore#703)
I have used a TestFixture to encapsulate the creation/deletion of the database
NB : Domain tests could be converted to use InMemoryStorage for faster run
@nfaugout-lucca
Copy link
Contributor Author

@Poltuu je me doutais bien que ça ne passerait pas en l'état ;)

J'ai fait un effort de refacto, tous les tests sont en inMemory désormais, seul 1 test, celui qui m'intéressait, est en localdb.

D'ailleurs j'ai découvert grâce à ce test que le OrderBy natif sur les Guid ne fonctionne pas. Je vais faire une Issue pour ce sujet en particulier.

@nfaugout-lucca nfaugout-lucca merged commit 9ae7e04 into master Oct 3, 2018
@nfaugout-lucca nfaugout-lucca deleted the REF.Tests branch October 3, 2018 13:37
Poltuu referenced this pull request Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants