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

Mocking layer missing but still referenced in Cargo.toml #36

Closed
2 tasks
Dessix opened this issue Mar 22, 2022 · 13 comments
Closed
2 tasks

Mocking layer missing but still referenced in Cargo.toml #36

Dessix opened this issue Mar 22, 2022 · 13 comments
Labels
enhancement New feature or request

Comments

@Dessix
Copy link

Dessix commented Mar 22, 2022

The mocks feature still exists in Cargo.toml but is no longer present in the crate.

It's mentioned in the prior repository's README.md, and was located under /src/mocks, but has since been removed.

Two items to address:

  • Remove the feature flag or mark it as something to be supported in the future
  • Suggest an alternative method of testing for developers using this library, so people don't reinvent the wheel or write tests with IO dependencies and race conditions on external servers
Dessix added a commit to microsoft/snocat that referenced this issue Mar 24, 2022
This was added to enable the feature only during dev-dependencies, but
it is unnecessary and a no-op. While it remains unanswered how to tweak
a dev-dependency without altering the version constraint, this has been
removed to allow publishing.

See aembke/fred.rs#36 for details about the
mocks layer being absent from current versions of the `fred` crate.
Dessix added a commit to microsoft/snocat that referenced this issue Mar 24, 2022
This was added to enable the feature only during dev-dependencies, but
it is unnecessary and a no-op. While it remains unanswered how to tweak
a dev-dependency without altering the version constraint, this has been
removed to allow publishing.

See aembke/fred.rs#36 for details about the
mocks layer being absent from current versions of the `fred` crate.
@aembke aembke mentioned this issue Apr 2, 2022
@aembke
Copy link
Owner

aembke commented Apr 14, 2022

@Dessix, I removed the mocking layer feature in the latest build, so that should help remove any confusion around this. In the meantime I'm still debating whether to bring it back, but it is a lot of work to build out that logic so I may not get around to it for a while.

Thanks for finding this though 👍 . When I merge the 5.0.0 PR I'll come back and update this issue.

@aembke
Copy link
Owner

aembke commented May 3, 2022

@Dessix this is in the main branch now and will be released as 5.0.0 shortly. I'm going to close this for now, but I'm planning on adding a wiki page for feature requests and I'll add the mocking layer to that list.

@aembke aembke closed this as completed May 3, 2022
@rezmuh
Copy link

rezmuh commented Jun 17, 2022

Hi @aembke i'm looking into this crate with a hope that there was a mock functionality built in. Given that this is removed now, what is the suggestion to mock Redis connections and some of the commands for testing purposes when using this library?

@aembke
Copy link
Owner

aembke commented Jun 17, 2022

Hi @rezmuh

Good question, but I don't have a great answer. I removed the mocking layer primarily because the Redis interface became huge and it didn't seem feasible to try to keep up with it on my own. You can always run redis with docker, or even leverage the testing scripts with this library if you'd like, but that would really only work with integration tests, and might be a hassle.

I've been debating adding the mocking layer back, but I wasn't sure where to start with the interface. If you're interested in this feature I can add back the plumbing behind the mock layer so that mocking a command boils down to writing one function and adding a line to a match statement. If the process were pretty plug-and-play like that would you have any interest in either submitting a PR or suggesting which commands to mock?

@aembke aembke reopened this Jun 17, 2022
@aembke aembke added the enhancement New feature or request label Jun 17, 2022
@rezmuh
Copy link

rezmuh commented Jun 21, 2022

Hi @aembke to be fair, as much as I would like to help by submitting the PR, I can't guarantee if time permits. But I am definitely willing to help where I can. I can give suggestions on which commands to mock as well (start with the basic is always good, right ;))

@rezmuh
Copy link

rezmuh commented Jun 21, 2022

Btw, since I'm trying this crate together with SeaORM, I find that their way of Mocking is quite easy to use: https://www.sea-ql.org/SeaORM/docs/write-test/mock . Not sure how you did it in the past but maybe there's something interesting there for you to look at.

@aembke
Copy link
Owner

aembke commented Jun 21, 2022

No problem. I managed to get most of the plumbing and basic commands done this weekend, and I was really just hoping you wouldn't say "I need the streams interface" since that's far and away the most complicated to mock.

I've been going with an approach that is a little more involved, but I'm debating switching to a model closer to what you linked. The problem for me historically with mocking Redis is that if you want to support the complicated commands with side effects then a mocking layer similar to what SeaORM does won't really work that well. The pubsub/keyspace, client, cluster, etc interfaces are tough to mock that way, as well as expirations and all that fun stuff. I'll probably stick to the approach I have (create a mock in-memory representation of the Redis database) but may change it in the next major release. The real test will come if/when somebody asks for a mock stream interface...

@rezmuh
Copy link

rezmuh commented Jun 22, 2022

Oh no, i'm not doing any streams. I only use RedisPool and pubsub (and some other basic commands)

@aembke
Copy link
Owner

aembke commented Jun 22, 2022

Ok great, that's pretty straightforward to implement. And one benefit of doing it the way I had started is that it'll also work with the SubscriberClient if you're using that.

@rezmuh
Copy link

rezmuh commented Sep 1, 2022

hi @aembke any update on the mocking functionality?

@aembke
Copy link
Owner

aembke commented Sep 1, 2022

Hi @rezmuh,

I'm in the middle of some major refactoring for 6.0.0 to better support the mock interface, and then I'm planning on adding the mocking layer back in 6.1.0. Realistically that'll take a few weeks, but it's on the top of the list.

@rezmuh
Copy link

rezmuh commented Sep 1, 2022

nice. looking forward to it. :)

@aembke
Copy link
Owner

aembke commented Dec 12, 2022

I just published 6.0.0-beta.1 to crates.io with a new mocking interface. It takes a different approach than the one in older versions of this crate but is much more sustainable long term since it doesn't require re-implementing big chunks of the server logic. If you have any feedback on it please let me know.

@aembke aembke closed this as completed Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants