-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
[dev] Extend tests #166
Comments
Hello, my name is Stefan, I'm from Germany and I'm using openHAB a few years now (since 2018, if I recall correctly). In the last weeks, I've followed your activities about openhab-js and explored this code base. I think it is time for me to give something back to this great community 😃 Writing some tests seems to be a good approach to get involved. Personally I like unit tests, as they give quick feedback, especially if they are fast and run during development. I could offer to have a look at the existing tests and try to contribute the missing parts to get an acceptable and helpfull code coverage. |
Hello Stefan, thanks for your answer.
If you’ve any questions about the codebase, please ask. I should be pretty familiar with the codebase.
Giving something back is always great! Thanks! All contributions are always welcome, no matters whether it’s actual code or docs improvements (you’ll see many contributors being active on many different repos).
That’s a strong point, FYI we are running the unit tests with a CI action on every PR and on every push to main.
Thanks for that offer, really appreciated. Feel free to start at any time and to ask anything that helps. A small point from my side: If it is possible to structure the unit test files the same way the actual codebase is structured, this would be great. Some additional information that might be helpful:
|
I saw that there are already a few tests written in Mocha. As these are only three tests at the moment, I started to use Jest to write the new ones. Personally I prefer Jest as testing framework, as the tests are straight forward and more readable, imho. To distinguish between the existing and the new tests, I used the naming
This is only a temporary solution, till I covered the complete (or nearly the complete) codebase with tests. Then we can move all tests to the same naming and switch back to only one npm script. @florian-h05, could you have a look at my current work and tell me, if I'm on the right track, please? Are you ok with my change to Jest? Here is the diff of my branch to the main of openhab-js: main...sfriedle:openhab-js:test-with-jest Another question, I want to ask: there are a few implementations of the abstract class
Can you tell me the use case for these providers? Do we need them anymore? If yes, this should be fixed. |
I would prefer:
Overall, it looks good. I have to admit that I've never used Jest or Mocha before, but Jest seems really intuitive to use. One question: I've seen that you added Can you please open a WIP PR? It is easier for me to comment and checkout PRs than branches on forks.
Yes, I have no personal preference regarding test frameworks. I'm happy when we have good test coverage. |
Unfortunately, no. I've never seen them in usage. As they do not work because their construction fails, but we haven't received any bug reports for them, I think that there is no one using them. |
Some history: I added some tests a long time ago solely to verify pieces of logic that I thought warranted tests at the time. I used Mocha because it was a framework that I was familiar with. As for the providers, I'm not sure of the state of it now, but the purpose of this class was to allow scripts to provider their own providers for openHAB. For example, all my things and items were defined in JS libs, then these were added to a custom provider which meant that they were populated in openHAB, and also referenced by other JS rule scripts (it's always been important to me to not have 'stringly typed' code, which tends to arise if you are defining things/items in the UI or definition files, then needing to reference them from scripts). You can see the original provider script here: https://github.com/jpg0/ohj/blob/master/provider.js, as well as it's consumption: https://github.com/jpg0/oh-config/blob/master/automation/lib/javascript/personal/node_modules/thingandlink/thingandlink.js Saying this, I don't use it any more, so I have no problem with it being removed. |
Thanks for the explanation.
@sfriedle Feel free to remove the providers. |
I use standardx, as it is common here. But standardx uses ESLint under the hood. For my tests I had to mark the global |
Relates to #166. * Add Jest dependencies and configuration. * Add tests for utils.js. * Add tests for triggers.js. * Add tests for cache.js. * Refactor mocking of global Java types. * Ignore .DS_Store. * Reorganize npm test scripts. * Move Jest mock functions. The Jest mock functions and spies are now closer to their callers. java.mock.js is now independent of Jest. * Add tests for osgi.js. * Add tests for actions.js. Signed-off-by: Stefan Friedle <[email protected]>
See #166 (comment). Signed-off-by: Florian Hotze <[email protected]>
Issue #164 and some other issues in the past occured, because a code change accidentally broke something.
To avoid this happening in the future, the tests should be extended in some way.
I see two options for this:
a) Extending the current unit tests at the
/tests
folder (I currently have no idea how they work).b) Introducing end to end tests that verify the functionality of the library against a real openHAB installation.
Personally, I see an advantage in real end to end tests, as we would not need to mock openHAB Java classes, and I think writing end to end tests is something that is already done in some PRs (by appending a test script for the new functionality).
@digitaldan WDYT?
The text was updated successfully, but these errors were encountered: