Skip to content

fix "Tests leave key in keyring", #1215#1584

Closed
swapnil290502 wants to merge 1 commit into
borgbase:masterfrom
swapnil290502:patch-1
Closed

fix "Tests leave key in keyring", #1215#1584
swapnil290502 wants to merge 1 commit into
borgbase:masterfrom
swapnil290502:patch-1

Conversation

@swapnil290502
Copy link
Copy Markdown

#1215 solved
removed key from keyring

borgbase#1215  solved
removed key from keyring
@m3nu
Copy link
Copy Markdown
Contributor

m3nu commented Feb 6, 2023

Thanks for the contribution!

I don't think this is what you want. It just breaks sorting keyrings.

To remove saved passwords after testing, you'll want to adjust our tests here: https://github.com/borgbase/vorta/tree/master/tests

Pytest fixtures can probably be used to make a helper that cleans up after itself. Maybe this was already solved elsewhere.

You'll also want to run our tests before opening a PR. They should all pass. This includes code formatting and functiona l tests.

Any other questions, we're here to help.

@m3nu m3nu marked this pull request as draft February 6, 2023 11:21
@swapnil290502
Copy link
Copy Markdown
Author

swapnil290502 commented Feb 6, 2023 via email

@real-yfprojects
Copy link
Copy Markdown
Collaborator

I am newbie to contribution I don't know how to contribute

Are you familiar with the basics of using git?
This would be the first step. Then setup you development environment as described in our guide. After that you can start coding.

I would start with adding a remove_password(self, service, repo_url) method to the abstract VortaKeyring and implementing that method in the subclasses VortaDarwinKeyring, VortaDBKeyring, VortaSecretStorageKeyring and VortaKWallet5Keyring. After that I would commit these changes. I would then write a second commit adjusting the tests to use the new method for removing keys they have created. We use pytest for testing. Make sure to read into how that works before this step.

@ThomasWaldmann ThomasWaldmann changed the title swapnil290502 fix "Tests leave key in keyring", #1215 Mar 4, 2023
@real-yfprojects
Copy link
Copy Markdown
Collaborator

Closing in favor of #1639

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