Provides Arquillian As Junit Rule to use it with other runner #147

dipak-pawar · 2017-12-18T14:36:34Z

Changes proposed in this pull request:

ClassRule ArquillianTestClass
MethodRule ArquillianTest
Tests with Test lifecycle event.
Documentation

Fixes: #145

…classrule and a rule

Adds validation for both ArquillianRule & ArquillianRunner to use at the same time

…into issue_145

bartoszmajsak · 2017-12-18T15:08:57Z

Thanks @dipak-pawar! Did you try to adjust some tests we have e.g. in Drone, Graphene or some of the showcases to see if there are any challenges? Another option would be to have it added to Chameleon as a version of SimpleDeploymentTestCase - but this time with rules instead. This will also require adjusting profiles in maven to execute both old and new test case

bartoszmajsak · 2017-12-18T15:30:43Z

I just checked on chameleon project and it works for such a simple deployment. You can see my changes here. Awesome!

What I would suggest doing next is to find some more sophisticated tests in arquillian-showcase and use those rules instead - somewhat an exploratory testing round.

In the meantime, I will review the code.

lordofthejars

I am really happy with all the changes and I don't see anything wrong, just one thing from the point of view of UX, do you think it should be better/possible to simplify:

@Rule
 public ArquillianTest arquillianTest = new ArquillianTest();
 
 @Rule
 public MethodRule testWatchman = MethodRuleChain.outer(arquillianTest).around(new TestWatchman());

into single @Rule so user does not need to set both?

dipak-pawar · 2017-12-19T14:34:40Z

@lordofthejars TestWatchman is not required for user to be added in tests. This is just an example if user has to use another MethodRule along with ArquillainTest Method Rule, then user has to make sure that ArquillianTest Rule is used as Outer Rule using RuleChain to work it correctly

lordofthejars · 2017-12-19T14:39:05Z

docs/rules.adoc

+
+IMPORTANT:
+However while using above Junit Rules you can't inject method scoped arquillian resources.
+In order to use any Method Rule with `ArquillianTest` Rule, you have to use it with RuleChain


Then maybe I should change In order to use any Method Rule to In order to use any other JUnit Method Rule I know it is normal but in my case, if I had read the whole information then I had understood better.

Also, I recommend to not put the MethodRuleChain example in the main example, just put it this part just after IMPORTANT thing, if not maybe user will copy paste it when it is not required.

Steve973 · 2017-12-19T15:09:32Z

Thanks for this, @dipak-pawar . It is a great addition, and with this change, it will make it easy to use Arquillian with Spock.

dipak-pawar · 2017-12-19T15:20:54Z

@Steve973: @ahus1 is original hero of this feature. Thanks to him from my side

smiklosovic · 2017-12-19T17:13:07Z

@dipak-pawar

Good job! I am starting to wonder what usecase I will use this for (not saying it does not have its usage, I am just excited where to try this out first haha)

Could you give more love to the code formatting? I dont know if I am just pedantic but it does not read like that proverbial wine, at least for me ... I am not sure why the code is kind of "squeezed" in order not to exceed 80 chars per line ...

@lordofthejars

Anyway, would it be possible to run Cube with this without Arquillian runner? Could I have some Spring test with this rule and Cube containers would be started under the hood?

bartoszmajsak · 2017-12-19T18:04:09Z

I dont know if I am just pedantic

You are not alone ;)

I am not sure why the code is kind of "squeezed" in order not to exceed 80 chars per line ..

We have IDEA and .editorconfig available here https://github.com/arquillian/org-settings/blob/master/code-settings/arquillian-idea-code-style.xml#L142. This is now set to 122 chars, but doesn't harm to make it longer we most of us use 16:9 screens these days anyway.

bartoszmajsak · 2017-12-19T18:15:01Z

Anyway, would it be possible to run Cube with this without Arquillian runner? Could I have some Spring test with this rule and Cube containers would be started under the hood?

That was the whole idea behind this feature. I guess this is what you are looking for https://github.com/arquillian/arquillian-cube/pull/913/files#diff-aa0602ab39541b61bdbc68c44cd18b90R12

@smiklosovic if you can spend some time doing a code review as well that would be more than awesome :) But no pressure. Happy to see you around!

bartoszmajsak

Great job @ahus1 @dipak-pawar! I have few things I would like you to consider before we hit upstream.

bartoszmajsak · 2017-12-19T20:05:20Z

docs/rules.adoc

+=== Rules
+:icons: font
+
+If you want to use arquillian functionality with some other runner. You can use Arquillian with Junit Rule using `ArquillianTestClass` and `ArquillianTest` Rules.


How about

If you want to use Arquillian Testing Platform with other JUnit Runner, you can now use JUnit Rules - ArquillianTestClass and ArquillianTest - and happily let Arquillian do the heavy lifting for your tests.

bartoszmajsak · 2017-12-19T20:08:57Z

docs/rules.adoc

+If you want to use arquillian functionality with some other runner. You can use Arquillian with Junit Rule using `ArquillianTestClass` and `ArquillianTest` Rules.
+
+In order to use full functionality of Arquillian you have to use both the rules i.e. `ArquillianTestClass` and
+`ArquillianTest`.


What if I only use one? Shouldn't we require having both to have similar functionality to what the @RunWith(Arquillian.class) has to offer?

ok I will update comment

bartoszmajsak · 2017-12-19T20:09:23Z

docs/rules.adoc

+   {
+      Assert.assertNotNull(arquillianTest);
+   }
+}


Let's use some more realistic test and new code formatting too. This test just shows then new ArquillianTest() works ;)

bartoszmajsak · 2017-12-19T20:10:13Z

docs/rules.adoc

+----
+
+IMPORTANT:
+However while using above Junit Rules you can't inject method scoped arquillian resources.


Make sure to use Arquillian upper-case in all the cases.

bartoszmajsak · 2017-12-19T20:10:23Z

docs/rules.adoc

+
+IMPORTANT:
+However while using above Junit Rules you can't inject method scoped arquillian resources.
+In order to use any other JUnit Method Rule with `ArquillianTest` Rule, you have to use it with RuleChain


`RuleChain`

Maybe also worth to mention that unfortunately it is not possible to have one rule acting as both test class and test method rule junit-team/junit4#351 (comment)

bartoszmajsak · 2017-12-19T20:19:53Z

junit/core/src/main/java/org/jboss/arquillian/junit/State.java

@@ -88,6 +89,15 @@ static void runnerStarted() {
        lastCreatedRunner.set(lastCreatedRunner.get() + 1);
    }

+    static boolean hasAnyArquillianRule(org.junit.runners.model.TestClass testClass) {


As commented above - maybe we should always check if both are present to fail if not?

It's not possible to check it. I will update the documentation with limitation

bartoszmajsak · 2017-12-19T20:20:11Z

junit/core/src/test/java/org/jboss/arquillian/junit/ClassWithArquillianClassAndMethodRule.java

@@ -0,0 +1,70 @@
+/*
+ * JBoss, Home of Professional Open Source
+ * Copyright 2009, Red Hat Middleware LLC, and individual contributors


Can you update the year for all those classes if we stick to have it here?

bartoszmajsak · 2017-12-19T20:20:58Z

.../arquillian/junit/ClassWithArquillianClassAndMethodRuleWithExceptionInAfterAndAfterRule.java

+import static org.jboss.arquillian.junit.JUnitTestBaseClass.wasCalled;
+
+/*
+ * Predefined TestClass


Not sure what those comments are bringing to the overall code understanding - let's remove them from all those tests.

bartoszmajsak · 2017-12-19T20:21:29Z

...g/jboss/arquillian/junit/ClassWithArquillianClassAndMethodRuleWithExpectedExceptionRule.java

+
+    @Rule
+    public ArquillianTest arquillianTest = new ArquillianTest();
+    @Rule


bartoszmajsak · 2017-12-19T20:25:23Z

junit/core/src/test/java/org/jboss/arquillian/junit/MethodRuleChain.java

+import org.junit.runners.model.FrameworkMethod;
+import org.junit.runners.model.Statement;
+
+public class MethodRuleChain implements MethodRule {


Can't we simply use RuleChain here? I'm not sure why we need this class. Especially that the way how we use it will result in calling our rule twice as it's part of the test and only then added to this rule chain (and they evaluated again).

Currently ArquillianTest implements MethodRule. While RuleChain needs parameter of type TestRule. So we can't simply use RuleChain here. I will update the tests how should we use it as outer rule

Let's expose it as a regular class so other users can leverage it too and add helpful JavaDoc to it.

…lass

This reverts commit ad1d4a6.

adds javadoc

bartoszmajsak · 2017-12-20T09:21:10Z

Awesome work @dipak-pawar.

And kudos to @ahus1 for kicking this work off!

bartoszmajsak · 2017-12-20T12:10:47Z

Upstream 6dc0cf4! Thanks!

ahus1 · 2017-12-20T13:59:32Z

Thanks @dipak-pawar - kudos for taking this from a poof-of-concept to a shiny and documented feature! I'm buried in other work - I'm sorry that I wasn't able to participate in the tests and discussions!

ahus1 and others added 18 commits June 16, 2016 22:29

Extracting essential functionality from Arquillian JUnit runner to a …

3e992cb

…classrule and a rule

adapting a set of tests for the variant with a rule

d8065ca

Fixes failing tests

64b4a56

Adds validation for both ArquillianRule & ArquillianRunner to use at the same time

Renamed variables

411d490

Renamed ArquillianClass1 to ClassWithArquillianRunner

050c854

Renamed ArquillianClass2 to ClassWithArquillianClassAndMethodRule

ffe067d

Merge branch 'master' of https://github.com/arquillian/arquillian-core …

d5bcfe2

…into issue_145

Removes diamond operator

76860d3

Renamed Rules

946404a

Corrected comment spelling

763eb80

Renamed class to make it consistent

2e865cd

Adds documentation for Rule

93e53d4

coorected spelling for testclass

5a01a8c

Updated javadoc with changed rule name

694d310

Changed indentation to 4 spaces

bb10ada

Indentation with 4 spaces

929a08f

fixed failing tests

e9e95ee

Indentation with 4 spaces

0c37048

bartoszmajsak requested review from bartoszmajsak and lordofthejars December 18, 2017 16:19

lordofthejars reviewed Dec 19, 2017

View reviewed changes

jollytoad mentioned this pull request Dec 19, 2017

Testrunner does not like Spock 1.0 arquillian/arquillian-testrunner-spock#18

Closed

lordofthejars reviewed Dec 19, 2017

View reviewed changes

dipak-pawar mentioned this pull request Dec 19, 2017

Create Arquillian Junit Rules Examples arquillian/arquillian-showcase#26

Open

Modifies documentation

c323b5e

lordofthejars approved these changes Dec 19, 2017

View reviewed changes

This was referenced Dec 19, 2017

Write examples for new Arquillian JUnit Rule arquillian/arquillian-cube#912

Closed

Adds few example tests for Arquillian JUnit rule. arquillian/arquillian-graphene#176

Merged

Adds example for new JUnit Rules arquillian/arquillian-cube#913

Merged

bartoszmajsak requested changes Dec 19, 2017

View reviewed changes

dipak-pawar added 14 commits December 20, 2017 10:27

Removes comment from test classes

171dad7

Fixes formatting for testclass

72378f2

Updates documentation for new rules

7697dc2

Uses ArquillianTest as outer rule correctly

b61d323

Changed exception message to make it more readable.

6a85e82

Removes redundant code which throws exception if result failed

0b38cde

Corrects usage of MethodRuleChain

47d8f7e

Adds more readable message for exception

cb2b6f3

Adds tests for ArquillianTestMethod to use it without ArquillianTestC…

2ad20bb

…lass

Updates documentations for not enforcing user to define both rules

7f9b27c

Revert "Changed copyright year from 2009 to 2017"

67fd003

This reverts commit ad1d4a6.

Removes license header

ec4fbbd

moves MethodRuleChain to main

eac2088

adds javadoc

bartoszmajsak approved these changes Dec 20, 2017

View reviewed changes

bartoszmajsak pushed a commit that referenced this pull request Dec 20, 2017

feat(ARQ-1828): improves JUnit rules handling (#147)

6dc0cf4

Merge branch 'master' into issue_145

46b2f48

bartoszmajsak closed this Dec 20, 2017

MatousJobanek mentioned this pull request Jan 3, 2018

Provide Arquillian as JUnit Rule #145

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provides Arquillian As Junit Rule to use it with other runner #147

Provides Arquillian As Junit Rule to use it with other runner #147

dipak-pawar commented Dec 18, 2017

bartoszmajsak commented Dec 18, 2017 •

edited

Loading

bartoszmajsak commented Dec 18, 2017 •

edited

Loading

lordofthejars left a comment

dipak-pawar commented Dec 19, 2017

lordofthejars Dec 19, 2017

Steve973 commented Dec 19, 2017

dipak-pawar commented Dec 19, 2017

smiklosovic commented Dec 19, 2017 •

edited

Loading

bartoszmajsak commented Dec 19, 2017 •

edited

Loading

bartoszmajsak commented Dec 19, 2017 •

edited

Loading

bartoszmajsak left a comment

bartoszmajsak Dec 19, 2017

bartoszmajsak Dec 19, 2017

dipak-pawar Dec 20, 2017

bartoszmajsak Dec 19, 2017

bartoszmajsak Dec 19, 2017

bartoszmajsak Dec 19, 2017

bartoszmajsak Dec 19, 2017

bartoszmajsak Dec 19, 2017

dipak-pawar Dec 20, 2017

bartoszmajsak Dec 19, 2017

bartoszmajsak Dec 19, 2017

bartoszmajsak Dec 19, 2017

bartoszmajsak Dec 19, 2017

dipak-pawar Dec 20, 2017 •

edited

Loading

bartoszmajsak Dec 20, 2017

bartoszmajsak commented Dec 20, 2017

bartoszmajsak commented Dec 20, 2017

ahus1 commented Dec 20, 2017

Provides Arquillian As Junit Rule to use it with other runner #147

Provides Arquillian As Junit Rule to use it with other runner #147

Conversation

dipak-pawar commented Dec 18, 2017

Changes proposed in this pull request:

bartoszmajsak commented Dec 18, 2017 • edited Loading

bartoszmajsak commented Dec 18, 2017 • edited Loading

lordofthejars left a comment

Choose a reason for hiding this comment

dipak-pawar commented Dec 19, 2017

Choose a reason for hiding this comment

Steve973 commented Dec 19, 2017

dipak-pawar commented Dec 19, 2017

smiklosovic commented Dec 19, 2017 • edited Loading

bartoszmajsak commented Dec 19, 2017 • edited Loading

bartoszmajsak commented Dec 19, 2017 • edited Loading

bartoszmajsak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dipak-pawar Dec 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartoszmajsak commented Dec 20, 2017

bartoszmajsak commented Dec 20, 2017

ahus1 commented Dec 20, 2017

bartoszmajsak commented Dec 18, 2017 •

edited

Loading

bartoszmajsak commented Dec 18, 2017 •

edited

Loading

smiklosovic commented Dec 19, 2017 •

edited

Loading

bartoszmajsak commented Dec 19, 2017 •

edited

Loading

bartoszmajsak commented Dec 19, 2017 •

edited

Loading

dipak-pawar Dec 20, 2017 •

edited

Loading