Conversation
| room.name()); | ||
| stats.incrementTimer(timer); | ||
| if (!Objects.equals(room.name(), SMOKETEST_ROOM_NAME)) { | ||
| stats.incrementTimer(timer); |
There was a problem hiding this comment.
Since its test specific production code, let's minimize it's distraction and move the whole if block into a function incrementTimerOfSmokeTestRoom or something, so its only one line.
There was a problem hiding this comment.
But that’s not what it does! It increments the timers for all room except for the rest room.
| room.name()); | ||
| stats.incrementBreaktimer(breaktimer); | ||
|
|
||
| if (!Objects.equals(room.name(), SMOKETEST_ROOM_NAME)) { |
gregorriegler
left a comment
gregorriegler
left a comment
There was a problem hiding this comment.
I like cypress. First I thought it would be overkill to have cypress for monitoring. But now I see you did meaningful tests. I think it's fine.
|
I first really just thought about testing if the index page return 200 but then I it came to my mind that the timer heavily depends on JavaScript and that we do not have any good tests for that. So I thought some smoke tests that test the core functionality would be good to catch issues there. In my honest opinion we should later even think about writing more cypress test which can be added to the ci pipeline. |
2 participants
We should have our pipeline for releasing first, as we need to release this after the change. If we do not do this we will get false entries for the statistics