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

Allow grouping of describe blocks using @Test annotations so IDEs can provide selective test running at suite level #98

Open
zifnab87 opened this issue Mar 1, 2017 · 16 comments

Comments

@zifnab87
Copy link

zifnab87 commented Mar 1, 2017

How about supporting this hybrid approach? This way there won't be too much need of fit, fdescribes etc and people will be able to use Spectrum without losing anything - Not sure if this is related to #79

    @Test
    public void something_suite() {
        describe("something",() -> {
            it("somethning", () -> {
                Assert.assertEquals(1, 1);
            });
        });
    }

If this can be done somehow I will migrate all my tests to use Spectrum!

@ashleyfrieze
Copy link
Contributor

@zifnab87 this is the opposite paradigm to Spectrum. The idea with Spectrum is that you create a single map of your specification using describe, it and in the about-to-be-released version, given,when,then,feature and scenario.

So, grouping your tests into suites is already supported your something_suite above would just be another describe.

In the about-to-be-released version, you can use ignore as a keyword to mark suites or tests ignored, rather than xdescribe or xit. You can also tag the hierarchy and use either system properties or a call within the code to focus or disable tests with certain tags.

Migrating all your tests to use Spectrum has to have an advantage, and your suggestion must come from something you feel you can't presently do.

If your tests are really behavioural, then Spectrum is a good language for expressing them.

Perhaps you could post a snippet of the sort of tests you'd like to improve and we could see how they would expressed in native Spectrum style.

@zifnab87
Copy link
Author

zifnab87 commented Mar 1, 2017

Hi @ashleyfrieze! Thanks for the quick response! What I am trying to do is integrate it with IntelliJ so I can select and run a specific test without changing the code at all. So I was seeking a way to annotate specific describes.. As far as I know @Test can be put only over methods that's why I proposed this hybrid approach.. Could you foresee the implementation possibility of an intelliJ plugin that can detect all the describe/it blocks and run them hierarchically with a click or be able to jump on the specific line of code without the use of @Test?

As for my tests all of them are using the rule of 3A (Arrange,Act,Assert) so I have been writing my unit tests in a way that I believe can be converted to pure BDD merging a lot of them that have exactly the same Arrange parts. So yes I see a lot of value in BDD JUnit tests!

@ashleyfrieze
Copy link
Contributor

Well well well :)

You've picked on the thing which these sorts of custom test runners can't do. You'd have the same trouble using Cucumber and all the other RSpec/Jasmie clones out there.

So your request is something like - Could there be a specialist runner for Spectrum which is compatible with selective test running in IntelliJ at suite level

It kind of looks like this:

@RunWith(SpectrumMagicRunner.class)
public class MyTests {
    @Test
    public void suite1() {
        describe("one", () -> { ... });
    }

    @Test
    public void suite2() {
        describe("two", () -> {
           it("has a spec", () -> { ... });
        });
    }

}

And you'd expect that the JUnit tree, when looking at this in IntelliJ or Eclipse, might have the tree in it:

MyTests
  suite1
     ...
  suite2
    has a spec

And your IDE UI would let you run MyTests but also re-run suite1 or suite2.

@greghaskins would this be a desirable twist on Spectrum for you? I can think of where to start to try to make it.

@zifnab87
Copy link
Author

zifnab87 commented Mar 1, 2017

Exactly!! I hope other people share the same view with me :) Otherwise one would have to split those suits to two separate Test Classes which already are a grouping factor of our Java tests no matter what the test paradigm :) So with that suggestion people are free to be as pure BDD as Java currently supports or be more integrated with JUnit capabilities that if are not there it could discourage the use of BDD Runners like Spectrum due to upper management objections for example.

In your example above I would expect:

MyTests
  suite1
     one
       ....
  suite2
    two
       has a spec

with the ability to have multiple describes in @Test methods and not impact test coverage support and code navigation granularity at least on the level of methods. I know anything more fine-grained won't be supported out of the box in the IDEs..

@zifnab87 zifnab87 changed the title Allow grouping of desctribe blocks using @Test annotations Allow grouping of describe blocks using @Test annotations Mar 1, 2017
@zifnab87 zifnab87 changed the title Allow grouping of describe blocks using @Test annotations Allow grouping of describe blocks using @Test annotations so IDEs can provide selective test running at suite level Mar 1, 2017
@ashleyfrieze
Copy link
Contributor

it could discourage the use of BDD Runners like Spectrum due to upper management objections

If your upper management are deciding how you unit test, you're in more trouble than a choice of test runner :)

Have you considered using JUnit to group your specs?

Here's a solution

@RunWith(Enclosed.class) // junit native runner
public class GroupOfSpecs {

   @RunWith(Spectrum.class)
   public static class Group1 {{
       describe("...", () -> {}); //etc
   }}

   @RunWith(Spectrum.class)
   public static class Group2 {{
       describe("...", () -> {}); //etc
   }}
}

Unless I'm wrong, I think you can selectively run each of the inner classes. The Enclosed runner is a JUnit trick for composing suites from static inner classes.

Does that help?

@greghaskins
Copy link
Owner

@ashleyfrieze 's suggestion should get part of the way there @zifnab87, and it works out of the box. I've used it myself to only run a subset of tests at a time from my IDE. It may not even require the @RunWith(Enclosed.class) on the outer class depending on your setup (mine quick test didn't need it).

JUnit5 (#68) is probably a better long-term solution for this, since that platform provides better support for alternative test runners, whereas JUnit4 very much wants a class name and a method name (which are used both for display and filter/re-run purposes). At least IntelliJ has started on proper IDE support for JUnit5.

@zifnab87
Copy link
Author

zifnab87 commented Mar 1, 2017

Ha! Currently I am not under any management myself especially considering the technologies I've picked to work with :) What I was trying to say is that this thought would come in the majority of cases in the industry considering rewriting their tests that can be modeled with BDD.. Let me try the suggestion, although a bit hacky and see if that causes any problems I will let you know :) Does JUnit5 already support runners running parts of lambda functions?

@zifnab87
Copy link
Author

zifnab87 commented Mar 2, 2017

@greghaskins, @ashleyfrieze so I tried doing what you suggested.. Unfortunately a lot of my dependencies that are private would need to be put as public everywhere.. so all the inner classes can access those. Also I have @Autowired dependencies (#99) that can't be accessed from a static context, which would mean that I need to reinject all those in all inner classes. Worse @Before doesn't make sense anymore since I have to have the same beforeEach logic in each inner class just to be used once.. Finally since my code has two parent classes one for BDD called BDDTest and one for the rest called BaseTest

when I try to run a class that looks like that:

public class AttributeControllerTest extends BDDTest {
    @Autowired
    private IAttributeService attributeService;
    private GraphTraversalSource graph;
    private MockMvc mockMvc;

    public static class TestClass extends BDDTest {
        {
            describe("something", () -> {
                it("something", () -> {
                    Assert.assertEquals(1, 1);
                });
            }); //etc
        }
    }

it obviously ignores BDDTest

if I change the outer extends to be BaseTest like that:

public class AttributeControllerTest extends BaseTest {
    @Autowired
    private IAttributeService attributeService;
    private GraphTraversalSource graph;
    private MockMvc mockMvc;

    public static class TestClass extends BDDTest {
        {
            describe("something", () -> {
                it("something", () -> {
                    Assert.assertEquals(1, 1);
                });
            }); //etc
        }
    }

I can run the inner test .. but when I run all tests or the outer class I get initialization error - no runnable method found or something like that..

This approach that you suggested, unfortunately, doesn't seem to scale for real applications like mine.

@ashleyfrieze
Copy link
Contributor

You can't expect the instance properties of the outer class to have meaning when you're using it as a suite. Each inner class would need to be a fully functioning test, and should, therefore, have its own properties and @Before method etc. However, all of those could be put into an abstract base class, even one within the parent.

Warning - you're expecting @Autowired to work, which it won't in the release version of Spectrum. In the latest code (soon to be released), you could make it work by also adding the Spring JUnit @Rule and @ClassRule to your class.

@zifnab87
Copy link
Author

zifnab87 commented Mar 2, 2017

I am able to make @Autowired dependencies work already.. you mean they will stop working in next releases? I already have parent classes and it seems that things get very hacky.. @before in each of the inner classes would have the same code which I believe is bad..if they were methods instead @before would make sense..

@ashleyfrieze
Copy link
Contributor

Which version are you using now? Support for spring will improve not reduce as we move forward. Can you provide the code from your base classes? This feels hacky now because you are trying to push native junit features into Spectrum rather than use its own approach. E.g @before vs beforeEach or let

@zifnab87
Copy link
Author

zifnab87 commented Mar 2, 2017

sure! Keep in mind that have 130+ tests currently that are integration tests due to the fact that I run against an in memory graph database instead of disk-based one.. Unfortunately graph databases are not really unit testable..Regardless they are behavior oriented and I really liked the way that the tests can be written and grouped together compared to the old approach I have still..I am running 1.0.2. Below are the two parent classes. These are the global @Autowired dependencies to minimize the initialization clutter in all of my tests, I also have @Autowired dependencies per test.. As you can guess the application is quite a bit complicated

@RunWith(SpringRunner.class)
@Ignore
@SpringBootTest
public class BaseTest {

    private static boolean offlineTests = true;
    @Autowired
    protected DbC conf;
    @Autowired
    protected IGraphService graphService;
    @Autowired
    protected StorerProvider storerProvider;
    @Autowired
    protected FetcherProvider fetcherProvider;
    @Autowired
    protected EntityFixtureBuilder entityFixtureBuilder;
    @Autowired
    protected DateFixtureBuilder dateFixtureBuilder;
    @Autowired
    protected UserFixtureBuilder userFixtureBuilder;
    @Autowired
    protected NumberFixtureBuilder numberFixtureBuilder;
    @Autowired
    protected UrlFixtureBuilder urlFixtureBuilder;
    @Autowired
    protected TextFixtureBuilder textFixtureBuilder;
    @Autowired
    protected AttributeFixtureBuilder attributeFixtureBuilder;
    @Autowired
    protected ClauseListFixtureBuilder clauseListFixtureBuilder;
    @Autowired
    protected AttrGroupListFixtureBuilder attrGroupListFixtureBuilder;
    @Autowired
    protected ClauseFixtureFactory clauseFixtureFactory;
    @Autowired
    protected AttrGroupFixtureBuilder attrGroupFixtureBuilder;
    @Autowired
    protected GraphEdgeStepFixtureBuilder graphEdgeStepFixtureBuilder;

    @Before
    public void beforeTest() {

        if (offlineTests) {
            conf.setupOfflineEnvironment();
        } else {
            conf.setupTestEnvironment();
        }

        graphService.setupTestUsers();
     }
    @After
    public void afterTest() {
        graphService.deleteAllData();
    }
}
@RunWith(Spectrum.class)
@Ignore
@SpringBootTest
public class BDDTest {

    private static boolean offlineTests = true;

    @Autowired
    protected DbC conf;
    @Autowired
    protected IGraphService graphService;
    @Autowired
    protected StorerProvider storerProvider;
    @Autowired
    protected FetcherProvider fetcherProvider;
    @Autowired
    protected EntityFixtureBuilder entityFixtureBuilder;
    @Autowired
    protected DateFixtureBuilder dateFixtureBuilder;
    @Autowired
    protected UserFixtureBuilder userFixtureBuilder;
    @Autowired
    protected NumberFixtureBuilder numberFixtureBuilder;
    @Autowired
    protected UrlFixtureBuilder urlFixtureBuilder;
    @Autowired
    protected TextFixtureBuilder textFixtureBuilder;
    @Autowired
    protected AttributeFixtureBuilder attributeFixtureBuilder;
    @Autowired
    protected ClauseListFixtureBuilder clauseListFixtureBuilder;
    @Autowired
    protected AttrGroupListFixtureBuilder attrGroupListFixtureBuilder;
    @Autowired
    protected ClauseFixtureFactory clauseFixtureFactory;
    @Autowired
    protected AttrGroupFixtureBuilder attrGroupFixtureBuilder;
    @Autowired
    protected GraphEdgeStepFixtureBuilder graphEdgeStepFixtureBuilder;

    @Before
    public void beforeTest() {

        if (offlineTests) {
            conf.setupOfflineEnvironment();
        } else {
            conf.setupTestEnvironment();
        }
        graphService.setupTestUsers();
     }
    @After
    public void afterTest() {
        graphService.deleteAllData();
    }

    private TestContextManager testContextManager;

    public BDDTest() {
        try {
            this.testContextManager = new TestContextManager(getClass());
            this.testContextManager.prepareTestInstance(this);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

@ashleyfrieze
Copy link
Contributor

ashleyfrieze commented Mar 2, 2017

Ok....

In the current code (i.e. head of master) there are two ways you could integrate BDDTest as a base class so you wouldn't need to do much to have the spring test framework stuff in there. Your constructor of BDDTest is hacking Spring wiring into the class by using TestContextManager, which you're only setting up, not clearing down...

Here's how you can do it in the latest version (soon to be released).

Method 1

Remove your constructor from BDDTest and add both the Spring class rule and method rule to the class. See https://github.com/greghaskins/spectrum/blob/master/src/test/java/specs/SpringSpecJUnitStyle.java for an example.

Then make each of your test classes inherit BDDTest and the latest Spectrum will automagically use the @Before, @After and spring stuff.

Method 2

Consider your test eco system as an object. It's an object you want to be used by each of your tests. So rename BDDTest as something like GraphDBEcoystemMixin and make it have all the objects, rules, setup etc you'd want to be able to use it in tests.

Then use junitMixin as demonstrated here - https://github.com/greghaskins/spectrum/blob/master/src/test/java/specs/SpringSpecWithRuleClasses.java - which allows you to compose the object into your tests.

Both of the above are the way to bridge from JUnit boilerplate to Spectrum. This only solves the how do I have my JUnit and Spring stuff everywhere question. The answer to how do I selectively run suites is still break it into classes. Arguably, with the above techniques, you could have smaller spec classes, rather than lots in a single parent suite, but that's up to you.

@zifnab87
Copy link
Author

zifnab87 commented Mar 2, 2017

About Method 1: what happens when someone doesn't have any config like my case.. Spring / Spring Boot / Spring Boot Test is moving towards removing config files and going towards auto-detection of injectables based on the locaton of the SpringBootApplication

@ashleyfrieze
Copy link
Contributor

h2

Sorry - used wrong markup h2 == ##

The example shown is a classic configure by config object Spring example. Don't focus on that. The SpringClassRule of your version of Spring Test should be able to cope with the way you want to configure your test object.

@ashleyfrieze
Copy link
Contributor

I recommend that we close this @greghaskins as I think it goes against the nature of Spectrum.

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

3 participants