RFC: Allow Redis fixture to use `decode_responses` parameter

[CrossNox]

For the redis client, the user might prefer to use the decoded response. There's no way to pass kwargs or similar, and I thought this might be reasonable.

For most of my usecases, I'm setting that parameter and would like to expect the same from the fixture.

[DanCardin]

I'm perfectly happy with this also being a config option, as you have laid out. But it probably also makes sense to make it an argument to the fixture creation function: create_redis_fixture(decode_responses=False), which takes precedence over the config-level option.

Basically as you've defined it, it will apply to all create_redis_fixture calls, which might be what you want. but to me it also makes sense for a per-fixture option to exist also.

---

Also, if you could write a simple test demonstrating the option actually has the desired effect, i'd appreciate it. Otherwise, i might be able to do so next week.

[CrossNox]

@DanCardin thanks for the input!

Makes sense to have it per fixture, just pushed those changes. I modified all tests to avoid repeating them, parametrizing them with a fixture with decode_responses=False (the default, for backwards compatibility) and another with decode_responses=True. I've run pytest and everything was passing with -k test_redis.

There are a couple styling changes that come from running poetry run ruff check --fix, I can revert them if you'd like.

[DanCardin]

to be clear, i'd have been fine with the config-level option also. but i also dont mind it not existing, up to you.

I suspect all the tests failing is because the tests are no longer directly depending on the fixture, which means they wont get mark.redis applied to them, which indicates they require docker.

I'd personally be happy to leave the existing set of tests as-is, and have 1 dedicated test to verify the option applies decoding

[CrossNox]

I'd personally be happy to leave the existing set of tests as-is, and have 1 dedicated test to verify the option applies decoding

Ok! Let me revert those changes and check if it works on CI.

[CrossNox]

@DanCardin Does it look better now?

As for the config level option, could you please check if this follows the desired preference level of the lib?

decode_responses=decode_responses or pmr_redis_config.decode_responses,

[DanCardin]

As for the config level option, could you please check if this follows the desired preference level of the lib? Yea that's what I would have done

[coveralls]

Pull Request Test Coverage Report for Build 9503124059

Details

• 8 of 8 (100.0%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.04%) to 89.511%

---

Totals
Change from base Build 9457060900:	0.04%
Covered Lines:	1445
Relevant Lines:	1595

---

💛 - Coveralls

[CrossNox]

Should I merge? Do you need anything else from my side for a release?

[DanCardin]

Should be released in 2.12.0