Region tests #308

ludverst · 2023-04-27T15:03:51Z

Beschrijving & Testmethode

Deze testen testen op de verschillende scenario's voor het regio-model, waaronder geslaagde requests, niet-succesvolle requests vanwege autorisatie, niet-bestaande paden en onjuiste typen regio-id's.

Aanpassingen

Bug fix (non-breaking wijziging die een probleem oplost)
New feature (non-breaking wijziging met nieuwe functionaliteit)
Breaking change (bestaande functionaliteit breekt door deze aanpassingen)

Checklist

De code volgt de stijl en guidelines van dit project.
Vereist een aanpassing aan de documentatie.
De documentatie is gewijzigd.

Close #307

TheMessik

Deze test suite voelt onvolledig aan. Ik zie bijvoorbeeld geen testen die over de user_region gaan en ik zou deze integratie toch wel graag getest zien.

Het is een goede aanzet, maar graag iets verder denken.

TheMessik · 2023-05-02T11:18:10Z

api/__tests__/routes/region.test.ts

+        afterAll(() => {
+            app.close();
+        });


Deze afterAll werkt enkel binnen de "Successful requests" blok. Deze moet in de buitenste blok komen.

Er staat ook een afterAll in "Unsuccesful requests". Is het beter om maar 1 keer afterAll te doen in de buitenste block?

Het gaat er niet om maar 1x te doen.
In de afterAll staat app.close(). Nu blijkt dat de testen alsnog verder kunnen, blijkbaar dankzij de requests library, maar het is overbodig deze resource 2x vrij te geven. Het doet hier geen kwaad, maar we hoeven ons niet onnodig te herhalen

api/__tests__/routes/region.test.ts

TheMessik · 2023-05-02T11:21:23Z

api/__tests__/routes/region.test.ts

+                test("Find a nonexistent region", async () => {
+                    await runner.get({
+                        url: "/region/-1",
+                        expectedData: [notFoundResponse],
+                        statusCode: 404,
+                    });
+                });
+
+                test("Update a nonexistent region", async () => {
+                    await runner.get({
+                        url: "/region/0",
+                        expectedData: [notFoundResponse],
+                        statusCode: 404,
+                    });
+                });


Hetzelfde comment hier als in mijn review van Round tests. Ofwel neem deze testen samen, ofwel noem ze anders.

ludverst · 2023-05-03T15:24:51Z

TODO:

soft delete testen

TheMessik

Nog een paar kleine aanpassingen vereist.

api/__tests__/routes/region.test.ts

TheMessik · 2023-05-05T15:59:50Z

api/__tests__/routes/region.test.ts

@@ -176,8 +176,78 @@ describe("Region tests", () => {
            });
        });

-        afterAll(() => {
-            app.close();
+        describe("path for admin", () => {


Als je maar 1 test hebt, hoef je er geen nieuwe test suite met describe van maken. De code in beforeEach kan je simpelweg bovenaan je test plaatsen.

TheMessik · 2023-05-05T16:03:32Z

api/__tests__/routes/user_region.test.ts

+                    });
+                });
+            });
+            describe("Cannot reach any path as a student", () => {


Er ontbreken tests voor updateOne en getOne.

een student kan momenteel voor elke id getOne doen. Ik heb daar een issue voor aangemaakt #355. Ik zal in deze issue de testen aanvullen voor getOne

TheMessik · 2023-05-10T09:00:16Z

API testen falen wegens een bug in validators, het zou gefixt moeten worden door #382

All validators have been updated to correctly refer to the id in params, using the correct celebrate syntax

TheMessik · 2023-05-10T10:01:41Z

De bug bleek door #382 juist geintroduceerd te worden, dit is nu gefixt

codecov · 2023-05-10T10:03:13Z

Codecov Report

Merging #308 (e032b3b) into develop (c7146fd) will increase coverage by 1.89%.
The diff coverage is 100.00%.

❗ Current head e032b3b differs from pull request most recent head f7351d1. Consider uploading reports for the commit f7351d1 to get more accurate results

@@             Coverage Diff             @@
##           develop     #308      +/-   ##
===========================================
+ Coverage    83.34%   85.24%   +1.89%     
===========================================
  Files           41       41              
  Lines         1093     1098       +5     
  Branches       221      223       +2     
===========================================
+ Hits           911      936      +25     
+ Misses         147      135      -12     
+ Partials        35       27       -8

Impacted Files	Coverage Δ
api/src/routes/region.ts	`93.93% <100.00%> (+40.36%)`	⬆️
api/src/validators/action.validator.ts	`100.00% <100.00%> (ø)`
api/src/validators/address.validator.ts	`100.00% <100.00%> (ø)`
api/src/validators/building.validator.ts	`100.00% <100.00%> (ø)`
api/src/validators/garbage.validator.ts	`100.00% <100.00%> (ø)`
api/src/validators/mailtemplate.validator.ts	`100.00% <100.00%> (ø)`
api/src/validators/progress.validator.ts	`100.00% <100.00%> (ø)`
api/src/validators/region.validator.ts	`100.00% <100.00%> (ø)`
api/src/validators/round.validator.ts	`100.00% <100.00%> (ø)`
api/src/validators/round_building.validator.ts	`100.00% <100.00%> (ø)`
... and 3 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

TheMessik

Goede vooruitgang, nog een paar kleine aanpassingen vereist.

TheMessik · 2023-05-10T10:11:46Z

api/__tests__/routes/region.test.ts

+                { deleted: false, id: 3, name: "Region 3", users: [] },
+            ];
+            await runner.get({
+                url: "/region",


Deze test wordt als super student gerunt zonder hardDelete flag, dus er zal soft delete uitgevoerd worden. Verder vraag je niet de deleted velden specifiek op, dus je merkt ook niet dat er zaken soft deleted zijn.

TheMessik · 2023-05-10T10:16:11Z

api/__tests__/routes/user_region.test.ts

+            });
+        });
+
+        afterAll(() => {


Dit hoort hier niet te staan.

…-Trottoir-1 into api/test/route-region

feat: region tests

28ae0ee

ludverst added api Werking van onze REST API test Heeft te maken met testen van functionaliteit labels Apr 27, 2023

ludverst requested review from jenspots, ArnoutAllaert and TheMessik April 27, 2023 15:03

ludverst self-assigned this Apr 27, 2023

ludverst linked an issue Apr 27, 2023 that may be closed by this pull request

API: Testen voor region route #307

Closed

ArnoutAllaert previously approved these changes Apr 27, 2023

View reviewed changes

TheMessik suggested changes May 2, 2023

View reviewed changes

ludverst added 2 commits May 3, 2023 15:59

fix: change test

a68aa86

feat: user_region tests

285fb98

ludverst dismissed ArnoutAllaert’s stale review via 285fb98 May 3, 2023 15:21

fix: tsc

f6f3d86

jenspots marked this pull request as draft May 4, 2023 12:04

ludverst and others added 4 commits May 5, 2023 11:25

feat: soft delete

35e7edb

fix: tsc

90ddfc0

chore: afterall

e33185d

Merge branch 'develop' into api/test/route-region

d0b5c00

ludverst marked this pull request as ready for review May 5, 2023 09:41

ludverst requested a review from TheMessik May 5, 2023 09:42

TheMessik suggested changes May 5, 2023

View reviewed changes

ludverst and others added 3 commits May 5, 2023 19:31

chore: describe wegdoen

a48a9d3

feat: add patch test student

153f331

Merge branch 'develop' into api/test/route-region

4cdcf47

ludverst requested a review from TheMessik May 5, 2023 18:02

Merge branch 'develop' into api/test/route-region

5841bf0

Merge branch 'develop' into api/test/route-region

3ce5d8c

Update validators, add deleted field to region

96b58e1

All validators have been updated to correctly refer to the id in params, using the correct celebrate syntax

Merge branch 'develop' into api/test/route-region

4c8c8bd

TheMessik suggested changes May 10, 2023

View reviewed changes

ludverst and others added 6 commits May 10, 2023 15:04

chore:region, update delete test

ad1ccad

fix: afterAll

e32b6aa

chore: rename var

020173d

Merge branch 'develop' into api/test/route-region

e6a05b7

chore: rename var

b64529a

Merge branch 'api/test/route-region' of https://github.com/SELab-2/Dr…

f7351d1

…-Trottoir-1 into api/test/route-region

ludverst requested review from TheMessik and ArnoutAllaert May 10, 2023 13:26

jenspots approved these changes May 10, 2023

View reviewed changes

jenspots removed the request for review from ArnoutAllaert May 10, 2023 13:37

TheMessik approved these changes May 10, 2023

View reviewed changes

jenspots merged commit 6802276 into develop May 10, 2023

jenspots deleted the api/test/route-region branch May 10, 2023 13:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Region tests #308

Region tests #308

ludverst commented Apr 27, 2023

TheMessik left a comment •

edited

Loading

TheMessik May 2, 2023

ludverst May 3, 2023

TheMessik May 5, 2023

TheMessik May 2, 2023

ludverst commented May 3, 2023 •

edited

Loading

TheMessik left a comment

TheMessik May 5, 2023

TheMessik May 5, 2023

ludverst May 5, 2023

TheMessik commented May 10, 2023 •

edited

Loading

TheMessik commented May 10, 2023

codecov bot commented May 10, 2023 •

edited

Loading

TheMessik left a comment •

edited

Loading

TheMessik May 10, 2023

TheMessik May 10, 2023

Region tests #308

Region tests #308

Conversation

ludverst commented Apr 27, 2023

Beschrijving & Testmethode

Aanpassingen

Checklist

TheMessik left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ludverst commented May 3, 2023 • edited Loading

TheMessik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheMessik commented May 10, 2023 • edited Loading

TheMessik commented May 10, 2023

codecov bot commented May 10, 2023 • edited Loading

Codecov Report

TheMessik left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheMessik left a comment •

edited

Loading

ludverst commented May 3, 2023 •

edited

Loading

TheMessik commented May 10, 2023 •

edited

Loading

codecov bot commented May 10, 2023 •

edited

Loading

TheMessik left a comment •

edited

Loading