chore(tests): add unit tests for js/modules #180
Conversation
mellodev
changed the title
mellodev
changed the title
mellodev
changed the title
There was a problem hiding this comment.
I suspect these are from the AI generations but far too much of these tests mock major functionality. I am not sure what they are testing with the level of mocking. I would rather properly intercept HTTP requests and not mock anything else. The app should fully function in testing so long HTTP requests to the firmware are mocked.
| } | ||
| }, | ||
| lang: 'en' | ||
| }; |
There was a problem hiding this comment.
Instead of overwriting the current session in the test, I think I would prefer to change the mocked payload (sinon) for the respective changes. For this example, the /jp and /jo responses would be different to match this test criteria. I think this avoids possible differences in what we expect versus what the firmware sends (also sets us up to support testing different firmware versions).
| }; | ||
| OSApp.currentDevice = { isMetric: false }; | ||
| OSApp.Language = { _: function(key) { return key; } }; | ||
| OSApp.Utils = { pad: function(num) { return ('0' + num).slice(-2); } }; |
There was a problem hiding this comment.
I don't think we should be mocking these functions. Why don't we just use the method itself?
| } | ||
| }; | ||
| setTimeout = function(func, delay) {}; | ||
| clearTimeout = function(timeout) {}; |
There was a problem hiding this comment.
This type of mocking doesn't make much sense to me. Why do we need to mock set and clear timeout?
| }; | ||
|
|
||
| // Mock OSApp.Errors.showError | ||
| originalShowError = OSApp.Errors.showError; |
There was a problem hiding this comment.
I don't think we should be mocking all of this (same comments so will stop making these).
Changes Proposed
/test/tests/modulesfolderskip.OSApp.Firmware.versionCompare!)npm testthen opentest/reports/Chrome xx/index.htmlwith your browser (See below)Demo Video or Screenshots
Coverage with this PR. Nowhere near the >= 80% I was hoping for, but at least it's a start!
