jmockit / jmockit1

Advanced Java library for integration testing, mocking, faking, and code coverage
Other
465 stars 240 forks source link

Invalid conditional statement inside expectation block #123

Closed henryken closed 9 years ago

henryken commented 9 years ago

Hi,

I'm getting IllegalArgumentException "Invalid conditional statement inside expectation block" when having an if statement inside my expectation block. It was working in v1.9 and when upgrading to 1.14, it throws me this Exception.

private void setUpExpectations(final SetUpNewServerFixture fixture, final boolean domainNameExists) {
        new NonStrictExpectations() {{
            trialServerRepository.domainNameExists(fixture.domainName);
            result = domainNameExists;

            if (!domainNameExists) {
                final TrialServer trialServer = createDefaultTrialServer(fixture.companyDomain,
                                                                         fixture.numOfTrialServersInDomain);
                trialServerRepository.save(withAny(trialServer));
                result = trialServer;
            }
        }};
    }
rliesenfeld commented 9 years ago

Yes, and this is as intended, to avoid tests getting too complicated when recording expectations. A full test was not shown, but it looks to me that recording the specific expectations directly would be better in this case.

henryken commented 9 years ago

Ok, wasn't aware that this is an intended change. Will modify my test expectations as necessary. Thanks.

hellonico commented 9 years ago

I have a question related to this topic, since we came down the same route.

We used to have fixtures, and were using the loop inside the Expectations block to check for each set of data. Is there any work around or recommended way to achieve the same ?

As a side note, we also tried to integrate jmockit with feed4junit, but without success ; each is somehow loosing access to the @Source element.

rliesenfeld commented 9 years ago

Yes, there are other ways to write the test. If you can show an example test that used a loop, I can show how it can be rewritten.

roelensw commented 9 years ago

Had the same problem and used a Delegate to fix it. See https://jmockit.googlecode.com/svn-history/r2056/trunk/www/tutorial/BehaviorBasedTesting.html#delegates

vlsi commented 9 years ago

@rliesenfeld , can you please look at this test code and suggest "how to rewrite without a loop"?

Test setup is as follows: we create DumpFileManager with capacity of 50000L. Then we send .fileRotated(dumpFile, null); to it. The size of each file is 10000L.

The expectation is DumpFileManager would hold the latest 5 files: the first five result in dumpFileLog.writeAddition, then manager should start to delete old files, so we expect writeAddition to be followed by deleteFile and writeDeletion.

Well, I can unroll those for-loops manually, however it would look like a workaround rather than "suggested way of writing test". "Loop unrolling" is manual, so it is prone to copy&paste errors.

The test did work as expected in jmockit 1.5

@Test
public void testFileRotatedWithDeletion(
        @Mocked final DumpFileLog dumpFileLog,
        @Mocked final FileDeleter fileDeleter
) {
    final List<DumpFile> expected = new LinkedList<DumpFile>();
    for (int i = 0; i < 10; i++) {
        DumpFile file = new DumpFile(
                String.format(root + "2015/03/11/1426083986775/calls/" + NUMBER_FORMAT.format(i + 1) + ".gz")
                , 10000L
                , 0L
        );
        expected.add(file);
    }

    new Expectations() {
        {
            new FileDeleter(); // one instance is expected

            new DumpFileLog(new File(root, DumpFileLog.DEFAULT_NAME)); // one instance is expected
            dumpFileLog.parseIfPresent();
            result = new LinkedList<DumpFile>();

            for (int i = 0; i < 5; i++) {
                DumpFile file = expected.get(i);
                dumpFileLog.writeAddition(file);
            }
            int lag = 5;
            for (int i = lag; i < 10; i++) {
                DumpFile addedFile = expected.get(i);
                dumpFileLog.writeAddition(addedFile);
                DumpFile deletedFile = expected.get(i - lag);
                fileDeleter.deleteFile(deletedFile);
                result = true;
                dumpFileLog.writeDeletion(deletedFile);
            }
        }
    };

    DumpFileManager dumpFileManager = new DumpFileManager(
            0L /* no age limits */
            , 50000L /* 20K */
            , root
            , false /* read from file */
    );
    FileRotatedListener listener = dumpFileManager.getFileRotatedListener();
    for (DumpFile dumpFile : expected) {
        listener.fileRotated(dumpFile, null);
    }
}
caillette commented 9 years ago

This change prevents me to upgrade from Mockito 1.12. Please tell me how I could remove the conditional. downwardDuty is the mock.

          new FullVerifications() { {
            downwardDuty.newProduct( ( Designator.Downward ) any, product ) ;
            for( final ServerModelFixture.SessionfulUser sessionfulUser : sessionfulUsers ) {
              if( sessionfulUser != firstUser ) {
                downwardDuty.newProduct( ( Designator.Downward ) any, product ) ;
              }
            }
          } } ;
rliesenfeld commented 9 years ago

Simple enough:

    int numberOfSessionfulUsersExceptTheFirst = 
        sessionfulUsers.size() - <number of times "firstUser" appears in the list>;

    // Call the SUT.

    new FullVerifications() {{
        downwardDuty.newProduct((Downward) any, product);
        times = 1 + numberOfSessionfulUsersExceptTheFirst;
    }};
caillette commented 9 years ago

How didn't find it? Thank you.

rchapman081068 commented 8 years ago

@rliesenfeld can you reply to @vlsi ?

new Expectations() {
    {
        ....
        for (int i = lag; i < 10; i++) {
            DumpFile addedFile = expected.get(i);
            dumpFileLog.writeAddition(addedFile);
            DumpFile deletedFile = expected.get(i - lag);
            fileDeleter.deleteFile(deletedFile);
            result = true;
            dumpFileLog.writeDeletion(deletedFile);
        }
    }
};
dayanand-ar commented 8 years ago

Can somebody explain how to achieve the above expectations? Basically, a sub-block of expectation has to be repeated N number of times.

rliesenfeld commented 8 years ago

@dayanand-ar What you're looking for is the "withCapture(List)" argument matching method.

dayanand-ar commented 8 years ago

Thanks @rliesenfeld. Basically I was looking for - http://jmockit.org/api1x/mockit/Expectations.html#Expectations-java.lang.Integer-java.lang.Object...- My bad that I didn't look into the documentation properly.