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

How do you write unit tests involving keyring-rs? #94

Closed
moritzheiber opened this issue Aug 3, 2022 · 10 comments
Closed

How do you write unit tests involving keyring-rs? #94

moritzheiber opened this issue Aug 3, 2022 · 10 comments

Comments

@moritzheiber
Copy link

I was pondering on this question since before the 1.1.x releases and the refactored Platform and PlatformCredential data model.

Is it possible right now to write unit tests using the established mappers? Platform has not trait implementation, PlatformCredential also doesn't. On top of that, the implementation is "dynamically" chosen, depending on the operating system, which would mean I'd have to write three tests for every single function I'd want to see passing.

Is there any way to write tests involving keyring-rs right now (and if so, how?)? Or would this need some refactoring to allow for a more dynamic association of resources (e.g. provide your own Platform implementation, or PlatformCredential implementation.

@brotskydotcom brotskydotcom self-assigned this Aug 3, 2022
@brotskydotcom
Copy link
Collaborator

Hi @moritzheiber, I want to make sure that I understand your question. There are already some unit tests, both of the platform-independent functionality and the platform-specific functionality, built into the code. Are you hoping to do more of that kind of test, but from outside the module? Can you give me an example of what kind of test you are thinking of? Is there a reason why those tests might not be appropriate to submit in a PR that could be integrated into the module?

@moritzheiber
Copy link
Author

moritzheiber commented Aug 3, 2022

My apologies, in hindsight I realize I should’ve been more clear about my intentions. I meant to ask about how I can write code, while using keyring-rs, and write unit tests for my own code which is interfacing with keyring-rs.

Usually, other potential crates have traits I can implement myself, abstracting most of the interfaces, or allow for passing your own implementation of OS-dependent modules or variables.. I haven’t found the same kind of possibilities with keyring-rs though, and thus my question of how one would go about and write unit tests involving keyring-rs.

I hope that makes it clearer, and sorry for having wasted your time initially.

@moritzheiber moritzheiber changed the title How do you write unit tests for keyring-rs? How do you write unit tests involving keyring-rs? Aug 3, 2022
@brotskydotcom
Copy link
Collaborator

Ah! I get it! You want to mock keyring for use in your own tests! That's a terrific enhancement request, so I'm reclassifying this issue. It should be easy to do this by adding a "mock" pseudo-platform.

A question back to you: do you need persistence between runs of your tests? Doing in-memory mocking is trivial, but doing persistent mocking would require a simple database of some kind.

@moritzheiber
Copy link
Author

I do not believe persistence lends itself to the general idea of unit testing, i.e. the way the mock should work is that it gives back whatever you define as its double. It will mean a few extra steps between function calls, but overall that's a good price to pay for avoiding the complexity of stateful representations 🙂

@brotskydotcom
Copy link
Collaborator

That's fine if you are only reading mocked objects, but presumably mocks can be set as well as read (otherwise it's not a complete mock). So the question is: how long should a mock that has been set persist its value? From your reply above, I would assume that you aren't expecting mocks to retain their values between tests or between test runs, and that each mock should be a separate object that need not be Sync (and thus usable across tests), but in fact actual keychain entries retain their values in both cases, and their values are not thread-local.

Can you confirm that what you are looking for would be satisfied by simply attaching an Option<String> to each mocked Entry, so that the entry would behave by reading and setting that string (or failing with a NotFound error code if the string was None)?

@moritzheiber
Copy link
Author

Ideally, it would emulate whatever the actual Entry class gives you back in terms feedback or values, however, you would be able to set relevant responses yourself somehow.

Having the Entry or Keyring interface accept trait implementations would be another way to go, you could then just implement your own “TestKeyring” or “TestEntry”. That’s how I see commonly implemented with other crates.

@brotskydotcom
Copy link
Collaborator

On the one hand, you are asking for a platform-independent mocking facility. On the other hand, you are saying you want it to give back the same platform-specific errors that the Entry class returns? That doesn't seem like a consistent position to me. Can you specifically answer this question: do you need to test platform-specific error returns in your code?

Also: it's not clear to me what you mean by the Entry and Keyring "interfaces". They are not interfaces, they are concrete types with concrete constructors. Can you give an example of another crate that returns concrete types with platform-specific implementations that uses this "trait-based" approach to mocking you refer to?

@moritzheiber
Copy link
Author

If at all I’d want to define the error return values myself when constructing the mock perhaps? Makes it a little more universal.

As for the trait approach, the cryptic crate does this: https://docs.rs/cryptex/latest/cryptex/trait.KeyRing.html

@brotskydotcom
Copy link
Collaborator

brotskydotcom commented Aug 7, 2022

OK, thanks for the pointer to the Cryptex crate. That clarifies for me a lot of what you are asking about. Here's my take:

  1. The Cryptex crate is designed to take pluggable implementations of get_os_keyring on different platforms. That's why it's written in terms of traits, and why it has no platform-specific types. The simplest way for you to get exactly what you want would be to write a module that implements the DynKeyRing trait for Keyring objects. Then any Cryptex client would be a keyring client, and you would be able to write your own mocks for tests.
  2. The Keyring crate has its own implementations for each platform, which is why it returns platform-specific error details, and it is not meant to be extensible. Mocking for Keyring would need to be added to the crate directly, and would take the form of a concrete Entry structure with built-in settable storage (for both values and errors).

I think adding a Cryptex adapter and mocks to Keyring are both good ideas, and I could see each being housed in its own submodule. I'll plan on building both, eventually. Right now, however, I'm fairly busy with a different project, so if you are inclined to contribute either or both I will be happy to review and integrate them.

@brotskydotcom
Copy link
Collaborator

Hi @moritzheiber, I believe #97 (design comments welcome there) rolls up all the functionality you're requesting here. I've proposed an alpha 2.0 release that provides this in #98 (code review welcome there).

I'm going to close this issue in favor of #97.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants