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

No test coverage for oldValue in events #150

Open
testower opened this issue Mar 22, 2021 · 8 comments
Open

No test coverage for oldValue in events #150

testower opened this issue Mar 22, 2021 · 8 comments

Comments

@testower
Copy link

I just noticed that the redisson implementation doesn't implement oldValue in cache entry events, but it still passed the tck tests. I just had a look at the tests coverage for events, and it doesn't seem like it's covered. Should it be?

@cruftex
Copy link
Member

cruftex commented Mar 23, 2021

Does not implement or does not do? Did you set CacheEntryListenerConfiguration#isOldValueRequired()?
Which version of redisson and TCK are you referring to?

@testower
Copy link
Author

Redisson does not implement oldValue. It's confirmed by the maintainer.

@testower
Copy link
Author

My question isn't really about Redisson, but about the tck tests. It looks like they don't cover oldValue - and is that deliberate or an oversight?

@cruftex
Copy link
Member

cruftex commented Mar 23, 2021

Just checked it. The assertions were missing missing up to TCK 1.1.0. TCK 1.1.1 was released to add these checks.
Changes were done by @vbekiaris in
2cb42a3
a3f8292

I have not double checked these changes. Does redisson pass on TCK 1.1.1?

@testower
Copy link
Author

I suppose this (https://github.com/cruftex/jsr107-test-zoo/blob/master/redisson-V2-test/pom.xml#L10) means they have only run against 1.1.0. Thanks for pointing that out 👍

@cruftex
Copy link
Member

cruftex commented Mar 23, 2021

Oh. Maybe I should do an update on the "test-zoo". Feel free to send a PR. It seems there is not much interest lately.

A note: Passing the TCK does not mean the implementation is compliant to the spec or in other words: its not sufficient. The TCK tries to test a lot, but does not cover everything, e.g. the whole concurrent behavior is not covered.

@cruftex
Copy link
Member

cruftex commented Mar 31, 2021

@testower, why close?

@cruftex cruftex reopened this Mar 31, 2021
@testower
Copy link
Author

testower commented Apr 1, 2021

Sorry about that, I closed the issue because my question was about coverage of old value in events, which you have answered: these are now covered by tests. If you think the issue should remain open to update the test-zoo that's fine. I haven't looked closely at how to run these.

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

No branches or pull requests

2 participants