ignite-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anton Vinogradov (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (IGNITE-4211) Update Spring dependency to latest stable version
Date Thu, 23 Mar 2017 12:08:41 GMT

    [ https://issues.apache.org/jira/browse/IGNITE-4211?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15938171#comment-15938171
] 

Anton Vinogradov commented on IGNITE-4211:
------------------------------------------

> If you don't agree with my approach, and if the test of @Cachenable(sync=true) is enough
for you:
> Just write it, I will change the test and then it's done.
> Such simple task was delayed for a long time.

We have a the highest standards across the industry and everything should be fixed/implemented
in proper way.

Also each contribution shoud follow 
https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules
and 
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines

> it is very strange test

Next rule is: each contributor should figure it out how current solution work before proposing
replacement.

> I think we have to cover the all new code with unit tests

Correct, but we should not have 2 or more test approach it case one is ehough.

> And I think we have to tests the following cases:
> valueLoader parameter was called only once per same key; [1]
> if a cache contains nothing for a key, a result of valueLoader parameter will added to
the cache correctly; [2] 
> if valueLoader parameter throws some exceptions, the new get-method rethrows the excepted
type of exception; [3]

Sure, we have to. 
All these cases can_be/already covered by GridSpringCacheManagerSelfTest

1) this checks by a double call 
{nofornat}
assertEquals("value" + i, dynamicSvc.cacheable(i));
assertEquals("value" + i, dynamicSvc.cacheable(i));
{noformat}

and follow-up counter check
{nofornat}
assertEquals(1, dynamicSvc.called());
{noformat}

2) this checks by a call 
{nofornat}
assertEquals("value" + i, dynamicSvc.cacheable(i));
{noformat}

and follow-up counter check
{noformat}
assertEquals(1, c.size());

assertEquals("value" + 1, c.get(1));
{noformat}

3) value loader is an aop'ed @Cacheable method and it can throw only runtime exception.
And it's will be covered in proper way. 

{noformat}
java.lang.RuntimeException
	at org.apache.ignite.cache.spring.GridSpringDynamicCacheTestService.cacheable(GridSpringDynamicCacheTestService.java:43)
	at org.apache.ignite.cache.spring.GridSpringDynamicCacheTestService$$FastClassBySpringCGLIB$$63a1306.invoke(<generated>)
	at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:204)
	at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:717)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
	at org.springframework.cache.interceptor.CacheInterceptor$1.invoke(CacheInterceptor.java:52)
	at org.springframework.cache.interceptor.CacheAspectSupport.invokeOperation(CacheAspectSupport.java:299)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:332)
	at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:281)
	at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:61)
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
	at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:653)
	at org.apache.ignite.cache.spring.GridSpringDynamicCacheTestService$$EnhancerBySpringCGLIB$$7bd6b246.cacheable(<generated>)
	at org.apache.ignite.cache.spring.GridSpringCacheManagerSelfTest.testDynamicCache(GridSpringCacheManagerSelfTest.java:350)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at junit.framework.TestCase.runTest(TestCase.java:176)
{noformat}

> Just make that the any get-methods returned "null".
> What happens? - Nothing. Green light in the GridSpringCacheManagerSelfTest.

Please write an example showing this.

> I found the bug IGNITE-4823, when SpringCache was covered with unit-tests
Thanks for finding this! 
Unfortunatelly @CachePut annotation not cover this, so we have to check this method directly.

BTW, any ideas why it's not covered by @CachePut?

> Update Spring dependency to latest stable version
> -------------------------------------------------
>
>                 Key: IGNITE-4211
>                 URL: https://issues.apache.org/jira/browse/IGNITE-4211
>             Project: Ignite
>          Issue Type: Improvement
>          Components: build
>    Affects Versions: 1.7
>            Reporter: Sergey Kozlov
>            Assignee: Vyacheslav Daradur
>             Fix For: 2.0
>
>
> It seems the Spring dependency looks outdated for now. Apache Ignite still uses 4.1.0
released two years ago. Could we to update to latest stable version (4.3.4 at the moment)?



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message