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

ISPN-5338 Extend JSR-107 tests #3436

Closed
wants to merge 1 commit into from

Conversation

mcimbora
Copy link
Contributor

@mcimbora mcimbora commented May 6, 2015

Not being able to reopen the previous PR [1] after force push, I'm creating a new PR with tests for clustered JCache scenarios. I switched back to using TestNG & tried to utilize test support classes as suggested.

[1] #3350

@mcimbora mcimbora force-pushed the t_ISPN-5338 branch 2 times, most recently from 64e6ce9 to fecf0cf Compare May 14, 2015 09:56
@mcimbora
Copy link
Contributor Author

Rebased. Looks like JCacheTwoCachesExpirationTest.testExpiration is still not passing.

@gustavocoding
Copy link

It always passes for me, what errors are you getting?

@mcimbora
Copy link
Contributor Author

It's passing as it's overriden in child class (see remote module, JCacheTwoCachesExpirationTest:35). As for the outcome of the test, I'm getting the following failure:

JCacheTwoCachesExpirationTest>AbstractTwoCachesExpirationTest.testExpiration:37 expected [0] but found [1]

Looks like expired listener is not notified after an entry has expired. It's test with 2 clustered caches.

@mcimbora
Copy link
Contributor Author

I'll have to flip expected/actual values.

@gustavocoding
Copy link

@mcimbora fair enough, I actually did not look at the code 😄
Will check....

@gustavocoding
Copy link

@galderz can confirm, but I don't think expiration notification was ever supported in JCache. Even for the embedded test, removing assert cache1.get("key1") == null; would cause it to fail as well since the expiration event is fabricated in a custom interceptor org.infinispan.jcache.embedded.interceptor.ExpirationTrackingInterceptor, and requires the entry to be accessed

@galderz
Copy link
Member

galderz commented May 19, 2015

Indeed @gustavonalle is right, expiration events are not currently supported. This was an oversight, probably due to the fact that the TCK does not test those. @mcimbora Can you create a JIRA for this? Feel free to have a go as well if you fancy :)

@mcimbora
Copy link
Contributor Author

@galderz @gustavonalle Thanks for feedback. I created JIRA for this (ISPN-5482). Indeed, I was looking at some ways how to contribute and JCache stuff seems to be a good candidate (especially things not covered by TCK, as they're probably not top-priority, e.g. ISPN-5326, ISPN-5318). What about the expiration test? Should I exclude it from this PR for now, or leave as it is? I prefer the second option, at least it'll be obvious what should/shouldn't be working.

@gustavocoding
Copy link

@mcimbora please @ignore the expiration event tests from the remote JCache for the moment.
With relation to ISPN-5482, it will be doable for both remote and embedded after https://issues.jboss.org/browse/ISPN-694 , which is not far away
Thanks! 👍

@mcimbora
Copy link
Contributor Author

@gustavonalle Fixed & rebased.

@galderz
Copy link
Member

galderz commented May 22, 2015

Integrated, thanks @mcimbora :D

@galderz galderz closed this May 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants