yegor256 / qulice

Quality Police for Java projects: aggregator of Checkstyle and PMD
https://www.qulice.com
Other
301 stars 111 forks source link

Prohibit fields in test classes unless they are really necessary #551

Open mkordas opened 8 years ago

mkordas commented 8 years ago

According to http://www.yegor256.com/2015/05/25/unit-test-scaffolding.html it is bad idea to have fields in test classes, as they create coupling between tests. Exception from the rule are all fields that are annotated, e.g. @Rule or @Parameter, so generally any annotation should skip validation.

Check could be implemented either by PMD or Checkstyle rule.

Non-compliant code

class Test {
    public File tempFile;

    private Object mock = Mockito.mock(Object.class);

    @Test
    public void test() {
        tempFile.createNewFile();
    }
}

Compliant code:

@RunWith(Parameterized.class)
class AnotherTest {
    @Rule
    public TemporaryFolder folder= new TemporaryFolder();

    @Parameter
    public int parameter;

    @Test
    public void test() {
        Object mock = Mockito.mock(Object.class);
    }
}
krzyk commented 8 years ago

@mkordas please extend the first (invalid) example with a test method, so it will look more like the second one

mkordas commented 8 years ago

@krzyk code sample is updated

krzyk commented 8 years ago

@mkordas I forgot about other JUnit annotations, which we sometimes use (e.g. @Parameters), so let's generalize it that we should ignore all fields annotated with any JUnit annotation.

mkordas commented 8 years ago

@krzyk @Parameters is annotation for methods only: http://junit.org/apidocs/org/junit/runners/Parameterized.Parameters.html You probably meant annotations connected with Theories, right? https://github.com/junit-team/junit/tree/master/src/main/java/org/junit/experimental/theories

krzyk commented 8 years ago

@mkordas I added s :), I meant @Parameterized.Parameter, singular

mkordas commented 8 years ago

@krzyk now I see, description updated

krzyk commented 8 years ago

@davvd valid bug

krzyk commented 8 years ago

@davvd this is postponed

davvd commented 8 years ago

@davvd valid bug

@krzyk tag bug added to this issue

davvd commented 8 years ago

@davvd this is postponed

@krzyk thanks, I added "postponed" label

davvd commented 8 years ago

@davvd this is postponed

@krzyk someone else will help in this task, no problem at all

davvd commented 8 years ago

@mkordas thank you for reporting this, I added 15 mins to your acc, transaction 5683c3027d0be85794000073

krzyk commented 8 years ago

@davvd this is not postponed

krzyk commented 8 years ago

@davvd this is urgent

davvd commented 8 years ago

@davvd this is not postponed

@krzyk got it, "postponed" tag removed from here

davvd commented 8 years ago

@davvd this is urgent

@krzyk right, I added "urgent" label

davvd commented 8 years ago

@guiandmag can you please help? Keep in mind this. If you have any technical questions, don't hesitate to ask right here

The cost of this task is 30 mins (this is exactly how much will be paid, not less not more), when the task is done

krzyk commented 8 years ago

@guiandmag How is the work going?

davvd commented 8 years ago

@guiandmag you're working with this ticket for the last 15 days. If it is not closed within the next 48 hours, it will be re-assigned to someone else, see No Obligations principle. This article should help if you're stuck; added -30 to your rating, now it is equal to -120

davvd commented 8 years ago

@guiandmag it takes too long. Usually, we expect any task to be finished in less than a week. I'll assign someone else. Please stop working with it right now. See our no obligations principle; -60 to your rating, your total score is -180

krzyk commented 8 years ago

@davvd this is postponed

davvd commented 8 years ago

@davvd this is postponed

@krzyk got it, "postponed" label here

davvd commented 8 years ago

@davvd this is postponed

@krzyk got it, "urgent" tags removed from here

davvd commented 8 years ago

@davvd this is postponed

@krzyk someone else will help in this task, no problem at all