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

Tests seems not to run in isolation #16

Open
fappel opened this issue Oct 2, 2014 · 4 comments
Open

Tests seems not to run in isolation #16

fappel opened this issue Oct 2, 2014 · 4 comments
Assignees
Milestone

Comments

@fappel
Copy link

fappel commented Oct 2, 2014

The default JUnit runner isolate tests in a way that every test gets executed with a new test-case instance. This ensures a fresh fixture instance for each test, even if the developer uses an implicit setup shortcut.

It seems to me that Oleaster tests work always with the same fixture in such a case. This is because the test-case instance is reused for all test runs (see snippet below for a better understanding of the use-case).

This behavior might lead to problems which are difficult to understand. Although I am not a particular friend of this setup practice, I doubt that people will follow a do-not-use-this-kind-of-implicit-setup convention. Maybe it would be better to ensure this kind of isolation as JUnit does by default.

@RunWith( OleasterRunner.class )
public class OleasterTest {

  Object fixture = new Object();

  {  
    it( "first", () -> {
      System.out.println( "first: " + fixture.hashCode() );
    } );

    it( "second", () -> {
      System.out.println( "second: " + fixture.hashCode() );
    } );
  }
}
@mscharhag
Copy link
Owner

Thanks for bringing this up. You are right, all Oleaster tests use the same test instance at the moment.

The problem here is that lambda expressions are bound to the scope of the surrounding object. In order to run specs in complete isolation

  • a new test instance is required for each spec
  • the code of all surrounding suites needs to be executed for the new test instance

So far I think this would be a good thing. However, I need to think a bit more about the consequences.
I will look into this.

@mscharhag mscharhag added this to the 0.2.0 milestone Oct 2, 2014
@mscharhag mscharhag self-assigned this Mar 9, 2015
@marioplumbarius
Copy link

I've been using the framework for about 2-4 months now, and I haven't had any problems on implicit setup. Indeed, I haven't noticed that junit did this kind of setup. I've always been expliciting the setup of fixtures, so I can have total control over them.

Anyway, I agree that the framework should not change the default behaviour from junit (where possible).

@mscharhag
Copy link
Owner

Hi marioluan,

thanks for your feedback.

The current behavior is this:

public class MyTest {{
  // executed once, before the first test runs
  describe("suite", () -> {
    // executed once, before the first test runs
    beforeEach(() -> {
      // executed before each test
    });
    it("test", () -> {
      // executed once
    });
  });
}}

When creating a new instance of the test class for each test, all the surrounding describe() blocks need to be executed again.

So the new behavior would be:

public class MyTest {{
  // executed before each test
  describe("suite", () -> {
    // executed before each test
    beforeEach(() -> {
      // executed before each test
    });
    it("test", () -> {
      // executed once
    });
  });
}}

This means:

  • using beforeEach() should not be necessary in most cases. Instead the surrounding describe() block can be used. I think this is ok, but it is different to Jasmine's behaviour.
  • There is no way to run initialization code once (before the first test case). I think the best solution to this is to support JUnits @BeforeClass and @AfterClass annotations in Oleaster tests.
  • It breaks compatibility with test written for older Oleaster versions. While I don't like this one, I think it is ok. Oleaster is still in an early version.

Feel free to share your opinions.

@ghost
Copy link

ghost commented Oct 22, 2015

It's definitely OK to break old tests written with the framework. At a very early stage, it's more important to refactor your code as you see fit to ensure that the framework is useful and usable.

Users should be aware of this risk, but you can make it more explicit in the readme if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants