-
Notifications
You must be signed in to change notification settings - Fork 13
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
Feature/post 0.3.0 followups #61
Conversation
- version updates - documentation updates - added release changes section to readme
@wh1337 still need the deploy and test scripts look a little off to me and happy to discuss. But really want/need the other packages released. Any chance? |
@toddb I can get those packaged up in a seperate PR shouldn't be an issue. It'll follow the same versioning as the main Novu package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed, LGTM
We are still seeing a failing test, which I would contribute to collision: @toddb review that and let me know if you agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still see failling tests, please review and just let me know if it's because of collision with the novu api
I'm not sure what "collision" means here. Sounds like an accident! The integration tests of the Integrations do not do any destruction or creation so they won't be creating race conditions. There's a design issue that needs to be understood.The test shows that the novu in-app provider is a special singleton—at least from my perspective of how to model across HTTP/REST. Hence why the original test had the assertion SingletonOrDefault rather than First. You'll probably also note that this is a Theory test and the change to first may be a better a approach, however, across all the providers when the test is completed. There are two improvements required in the code. The first is already logged as #56 because the test has a magic string—that was a placeholder that others are able to build on. The second is more a fundamental design problem of the way the Refit library is wrapped—there is no wrapping layer in the client code that can make cater for the practical impedance mismatch between the API and the calling code—in this case error handling that aren't necessarily exceptions but rather logic errors. Back to the test. So, the test assumes that a logical DELETE on a resource then allows for a POST. Hence it tests that the GET on the resource fails. It then does a passing POST. I managed to recreate the error (which didn't make sense) but now the tests don't. I added explicit lifecycle branches in the test code—I'm still a little at a lose to the underlying issue that looked like a timing issue. |
- also started adding EditData mapping extension methods
I have reworked the test and deploy scripts, documentation is inline in the scripts.
There is an issue within one of the integration tests meaning that it hangs. I have paused that phase but left other check and balances in place. |
…to be built through a pipeline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved, will get this published as v0.3.1
Github issues closed
0.3.1