test: [TECH-887] improve rules engine unit test coverage #3462
Conversation
(cherry picked from commit c86d002)
simonadomnisoru
changed the title
|
Hey @simonadomnisoru! I do however get a lot of warnings and actually some errors as well when running the tests locally.
These errors and warnings are not part of the tests we have in master. Is this something we need to address or is it fine as is? |
Hey @eirikhaugstulen, Warning and error logs are expected in the console. They are not shown in master because so far we mainly tested the "happy path". In this PR, I added a bunch of test cases where I deliberately tested with invalid data to make sure the input was properly validated and the expected errors were thrown. Let me know if you have any more questions. Thanks for the review! |
There was a problem hiding this comment.
Hi @simonadomnisoru! I only have two comments about some changes you made in the rules engine. I haven't looked at the tests you added, mostly because it seems like a frightfully big task to get the overview of what's been implemented and then try to figure out whether the current coverage is satisfactory. It might help with a little more information on the strategy you used to get to this point.
packages/rules-engine/src/processors/rulesEffectsProcessor/rulesEffectsProcessor.js
Show resolved
Hide resolved
packages/rules-engine/src/services/VariableService/helpers/structureEvents.js
Show resolved
Hide resolved
|
🎉 This PR is included in version 100.47.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
TECH-887
Tech summary
Improved the unit tests coverage for:
packages/rules-enginesrc/core_modules/capture-core/rulesfolderThe first commit is just a format of the test files. Therefore, I recommend that you take a look at the 296c1be commit to visualize the changes more easily.