-
Notifications
You must be signed in to change notification settings - Fork 192
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
Increase test coverage #539
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #539 +/- ##
==========================================
Coverage ? 90.04%
==========================================
Files ? 74
Lines ? 1858
Branches ? 0
==========================================
Hits ? 1673
Misses ? 185
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for making the effort of writing these tests!
@@ -25,7 +27,7 @@ class Index { | |||
name == other.name && | |||
tableName == other.tableName && | |||
unique == other.unique && | |||
columnNames == other.columnNames; | |||
const ListEquality<String>().equals(columnNames, other.columnNames); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏 Well spotted!
This PR pushes the test coverage over the magic 90% :)
Some errors from the change method processors were pulled out but the rest is pretty much just more tests.
I tried to tackle the biggest target in the codecov heatmap,
value_objects
, but while theequals
methods seem simple, thehashCode
methods are wrong in most cases, because thehashCode
of dartElement
s can be different even if the dart elements themselves are equal (==
). But the value objects also don't really benefit all that much from testing since they only have trivial methods left.So I took the next best target, error messages. Aside from those there are only two tests (database writer with daoGetters and entity processor with indices) that cover something else. I catched a single issue where the equals method of
Index
was not 100% correct(should only be an internal problem at the very most, if it mattered at all) but nothing apart from that.There is also one test that I had to set to
skip
because I could not figure out how to trigger that specific error message (EntityProcessorError.foreignKeyDoesNotReferenceEntity
). I can also remove that if there is no "solution" ;)