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

Invalid conditional statement inside expectation block #123

Closed
henryken opened this issue Jan 10, 2015 · 13 comments
Closed

Invalid conditional statement inside expectation block #123

henryken opened this issue Jan 10, 2015 · 13 comments
Labels

Comments

@henryken
Copy link

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
Copy link
Member

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
Copy link
Author

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

@hellonico
Copy link

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
Copy link
Member

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
Copy link

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
Copy link

vlsi commented Oct 6, 2015

@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
Copy link

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
Copy link
Member

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
Copy link

How didn't find it? Thank you.

@rchapman081068
Copy link

@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
Copy link

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

@rliesenfeld
Copy link
Member

rliesenfeld commented May 25, 2016

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

@dayanand-ar
Copy link

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.

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

No branches or pull requests

8 participants