Improve ValidateDataStructure error reporting #2366
Conversation
|
@kimkulling could I get some feedback on this, especially with regards to whether there should be a unit test for the validator? Thanks for all the effort you guys have put into the library, it's fantastic. |
|
This pull request introduces 6 alerts when merging 80dffb6 into 5c900d6 - view on LGTM.com new alerts:
Comment posted by LGTM.com |
* Fixed warnings introduced by last commit (hopefully) * Fixed case fallthrough (due to exception flow, it didn't make a practical difference, but hopefully will remove a warning) * Minor formatting consistency improvements
|
This pull request introduces 2 alerts when merging 7bb1303 into 5c900d6 - view on LGTM.com new alerts:
Comment posted by LGTM.com |
|
This pull request introduces 2 alerts and fixes 1 when merging 27dc922 into 5c900d6 - view on LGTM.com new alerts:
fixed alerts:
Comment posted by LGTM.com |
|
This pull request fixes 1 alert when merging 5fd9789 into 5c900d6 - view on LGTM.com fixed alerts:
Comment posted by LGTM.com |
|
Again, I'm happy to start adding unit tests for ValidateDSProcess, as mentioned in the original PR above, but given that I haven't gotten a response and it has no warnings, I'm marking this ready for review. |
Cgettys
changed the title
|
Hi, sorry for my late response. I was busy in my daytime-job. I already start to do a review, but I haven't finished this until now. Thanks a lot for your help! Kim |
|
This pull request fixes 1 alert when merging 90dbd8f into 152cdde - view on LGTM.com fixed alerts:
Comment posted by LGTM.com |
|
Merged, thanks a lot for the contribution. |
I was debugging some code and noticed that the ValidationDSProcess code could give me more info than it was with a few minor tweaks (e.g. telling me what the name of the aiNode missing a parent is, etc). I thought these improvements might be handy to others.
I did notice that there doesn't appear to be a unit test for the validation code and was considering starting one. I was wondering if that'd be useful. I'm thinking something along the lines of constructing objects with exactly 1 check that should fail, and then running those objects through the validator to make sure they do. It'd be a fair bit of work, and I'm not sure I'd do all of it at once, but given that it looks like to me that basically every unit test depends on that file to make sure there are no errors, it might be effort well spent. I'm happy to start one that at a minimum covers the cases I modified if that'd be helpful.