Skip to content
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

Add Mocked Tests: Volume Actions #214

Merged
merged 3 commits into from
Oct 2, 2023
Merged

Conversation

shivam-Purohit
Copy link
Contributor

refers issue #203

Add mocked tests for volume action.
Add tests for volume_actions.get, volume_actions.list, volume_actions.post, volume_actions.post_by_id

@shivam-Purohit
Copy link
Contributor Author

shivam-Purohit commented Sep 30, 2023

@danaelhe am I on the right path?
edit: I added all the tests but am confused about the volume ID. Also the python code at volumes_actions.get is wrong, it should also have the action_id parameter passed.

Copy link
Contributor

@work-mohit work-mohit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shivam-Purohit Yupp doing great !

Copy link
Contributor

@work-mohit work-mohit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@shivam-Purohit shivam-Purohit marked this pull request as ready for review October 1, 2023 05:23
@danaelhe
Copy link
Member

danaelhe commented Oct 2, 2023

Looks great!! Apologies, I should've included in the instructions but we lint our tests with the python package black. Currently, the "lint" test is failing for this PR but it's a super easy fix. If you run: poetry run black tests while in the pydo directory, it will automatically reformat the files and the "lint" test will pass. So just run that command and push up the changes and the lint tests will pass 👍

danaelhe
danaelhe previously approved these changes Oct 2, 2023
Copy link
Member

@danaelhe danaelhe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks FANTASTIC! Will merge once a commit has been pushed to fix lint tests pass (see comment from above)!

@shivam-Purohit
Copy link
Contributor Author

shivam-Purohit commented Oct 2, 2023

@danaelhe I pushed the changes. Is it okay now?
Also, can't we automate it with something like github precommit? It will be a lot easier.

@work-mohit
Copy link
Contributor

@danaelhe I pushed the changes. Is it okay now? Also, can't we automate it with something like github precommit? It will be a lot easier.

I was thinking about the same.

@danaelhe
Copy link
Member

danaelhe commented Oct 2, 2023

@danaelhe I pushed the changes. Is it okay now? Also, can't we automate it with something like github precommit? It will be a lot easier.

That's a great idea. I'll write up a github issue for it. Thanks!

@danaelhe danaelhe merged commit 7d8bbf2 into digitalocean:main Oct 2, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants