feat: add blueprint methods #90
Conversation
|
hey @ryshu, how to proceed with this now, can you brief me a bit? |
|
I updated the code according to the linter, can you re-run the tests and let me know if there are still any issues? |
|
Hi, you can run all pipelines tests on your own computer if you want. Still missing all tests, pipelines will fail. 100÷ coverage is required to pass pipelines. |
|
Hello, I have added the unittests for the blueprint methods, test coverage should be 100+ now. |
|
hey @ryshu, can you review the PR? I am facing some issues with implementation of a unit test, one of them works fine but the other one is not passing. |
There was a problem hiding this comment.
When I look at the history, I see that the commits you make don't follow the naming convention.
I also observe that the merge request is targeted towards the "main" branch while we use the "alpha" branch for current developments.
Since all of these points are covered in the contribution guidelines, I would like you to take a look at them before asking for my help. https://github.com/novuhq/novu-python/blob/main/CONTRIBUTING.rst
For the unit test part, the unit test fails and shows the error you are making. Since it is enough to read the result to understand the error, I don't see where I can be useful.
Result doesn't match your expectation.
FYI, for debug purpose only (should never be comited), you can use self.maxDiff = None before the test to see the untruncated message. https://docs.python.org/3/library/unittest.html#unittest.TestCase.maxDiff
tests/api/test_blueprint.py
Outdated
| ) | ||
| cls.expected_grouped_blueprint_dto = GroupedBlueprintDto( | ||
| category="example_category", | ||
| general=[{}], |
There was a problem hiding this comment.
API return and expectation aren't the same here. You expect and empty general when the API return blueprint DTO
|
@Yash-sudo-web Do you want me to finish your MR? The unit tests don't seem to be too complex for me to solve and we can plan a new alpha quickly. |
|
hey, @ryshu yes you can finish the MR, thanks. |
|
@Cliftonz Hi, also ready for rebase and merge |
Addresses Issue #79