++1 from my side. I guess it will be a more reliable way for works with properties in the tests. From time to time somebody forgot clear property after test and next tests may be failed or flaky failed completion.
On Thu, Feb 14, 2019 at 6:56 PM Dmitriy Pavlov <[email protected]> wrote: > I find it absolutely positive and modern approach and good contribution. > Count on my support if you will need any assistance with applying this > patch. > > чт, 14 февр. 2019 г. в 18:53, Ivan Bessonov <[email protected]>: > > > Hello Igniters, > > > > I'd like to discuss the way we treat system properties in our test > > codebase. > > It's a common pattern where we set system property in > "beforeTestsStarted" > > and clear it in "afterTestsStopped". Purest example that I've found is > > class > > "RedisProtocolGetAllAsArrayTest". > > > > There are similar things with "beforeTest"/"afterTest" or huge > > "try/finally" blocks > > right in test methods. > > > > I think that all this code can be avoided and solution might look like > > this: > > > > @Test > > @WithSystemProperty(key = IGNITE_PROPERTY_NAME, value = "true") > > public void testSomething() throws Exception { > > ... > > } > > > > Same annotation might be used on class, this way new system property will > > be applied to all test methods in the class. > > > > I already created the issue for this change [1] and PR with demo [2]. It > > contains > > implementation of annotation processing and a few migrated tests. If you > > like > > the idea then I will migrate all the other tests on the same mechanism. > > > > What do you think? > > > > [1] https://issues.apache.org/jira/browse/IGNITE-11323 > > [2] https://github.com/apache/ignite/pull/6109 > > > > -- > > Sincerely yours, > > Ivan Bessonov > > >
