-
Notifications
You must be signed in to change notification settings - Fork 79
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
Datetime tester feeds optional params; need tests with only positional args #195
Comments
For example, These calls passed in LF 0.2.3, and the change didn't make any tests fail. We discovered the problem when a German-speaking Mycroft user tried it. |
I don't think this would be caught on any language since the tests always give an anchorDate
suggestion would be to throw in some tests without the need to set a specific date eg
|
Agreed. I think some/most of this still involves those JSON parsers, so it'll be more than a five-minute fix. Take a stab, if you like; if not, I'll throw it on the roadmap thingy. |
The standardization pass onparse_de.py
seems to have messed up certain cases that weren't detected until the update made it all the way to Mycroft-core.It's the datetime tester. The datetime tester has both the optional params populated in all languages. This inherently misses all nondefault invocations. Glaring oversight. At least seven programmers and four reviewers, including myself.
This ticket could be an adage about impostor syndrome, Dunning-Kruger, and the fictional nature of 100% test coverage.
The text was updated successfully, but these errors were encountered: