View Skipped Days In The Bar Chart #1736
Conversation
I noticed that the skipped days appear in the history chart but not in the bar chart so I modified the code adding a piece of code to pass the skipped days data from database to the view and drawing a bar with a slightly transparent color -to let it be different from the main column- accordingly.
Updated the tests to provide the data for skipped entries too.
|
|
||
| init { | ||
| component.axis = axis | ||
| component.series.add(series1) | ||
| component.skippedSeries.add(series2) |
There was a problem hiding this comment.
How come the test setup seems to change, but the assertions (the .png) do not? Are they just close enough such that the tests pass anyway, and should they be updated instead? What am I missing?
There was a problem hiding this comment.
After running the test multiple times with different cases, here is a final conclusion:
First, the test mechanism seems to ignore little differences between the expected image and the actual one, so yes, they are close enough such that the test pass as long as the difference value is less than 1.
Second, I think the test mechanism should be updated to take care of little difference, then according to that update, the expected image should be updated too. We may update it for now until we take care of the test in a different PR.
Here are some of the results I got from the test, feel free to try them on your own (each test case is provided with the difference value and the output image too):
-
the current expected image (the one we may update later):
[Imgur] -
the current test data (with skipped days):
[Imgur](current test data with skipped days)
[Imgur](output)
[Imgur](difference) -
the current test data (without skipped days):
[Imgur](current test data without skipped days)
[Imgur](output)
[Imgur](difference) -
a modified test data (accepted):
[Imgur](accepted modified test data)
[Imgur](output)
[Imgur](difference) -
a modified test data (rejected):
[Imgur](rejected modified test data)
[Imgur](output)
[Imgur](difference)
There was a problem hiding this comment.
I think it's better to update the expected images in the same PR, following https://github.com/iSoron/uhabits/blob/dev/docs/TEST.md#running-instrumented-tests, but you can also wait for @iSoron to chime in if you prefer.
There was a problem hiding this comment.
It's ok to wait. Updating the image takes no time anyway, So there's no need to rush.
There was a problem hiding this comment.
@hiqua Hey there! how are you doing? I hope you're fine. I just wanted to tell you that I will update the test image and request you to review this PR so we can finally merge it.
There was a problem hiding this comment.
@hiqua Test image is up-to-date and this PR is ready for a final review.
I updated the expected image in the test to take into account the skipped days as well as the completed days. I also changed the test input in BarChartTest.kt to let it better represent possible cases.
I noticed that the skipped days appear in the history chart but not in the bar chart so I modified the code adding a piece of code to pass the skipped days data from the database to the view and drawing a bar with a slightly transparent colour -to let it be different from the main column- accordingly.