-
Notifications
You must be signed in to change notification settings - Fork 362
Improve test command coverage for demo flow #381
Comments
Hey, @noaabarki I m a beginner, can u please guide me to work on this issue |
Hi @Kanika637, that's awesome, and of course! Do you have any specific questions that I can help you with in order to get started? Also, feel free to use our community slack channel for raising any questions. Specifically, I highly recommend you to assist @Abhra303, since he understands our project and wonderful contributor 👑 |
Sure @noaabarki I will be doing that... |
@Kanika637 good to know you're working on this :) Could you please wait for my PR (#410 ) to get merged? Because, the fixes for these issues ( #382, #383 and this issue itself) are related mainly to Let's see what @noaabarki says on this. |
Sure @Abhra303 I will wait for your PR to get merged first. |
I have made the changes in my local system...@Abhra303 did your PR get merged? |
Hi @Kanika637, no my PR is still waiting for review ( it may take two or three days to review it). But if you think you did all the required changes - feel free to make a PR. One of us (you and me) will do a force update on our branches if another's PR get merged :) |
Yeah, I think this can create some conflicts...So I will wait for your PR |
@noaabarki what would you say on this? |
Hey @Kanika637, continue our conversation in the Slack channel, I wouldn't wait, I believe the update will go smooth. To speed up the process, I'll try my best to review @Abhra303 PR today. |
I have made the PR for issue #381...Thank you soo much @noaabarki @Abhra303 @royhadad for being such supportive. |
Describe the solution you'd like
Add tests for demo flow in test command
datree test ~/.datree/k8s-demo.yaml
.Requirements
Golang basic level.
“How to implement” Suggestion
In
cmd/test/main_test.go
, add new test case:: run test with demo-path without/with additional args → should print expected output.The text was updated successfully, but these errors were encountered: