jMock

Allow assert of State-Machine

Details

  • Type: New Feature New Feature
  • Status: Resolved Resolved
  • Priority: Trivial Trivial
  • Resolution: Fixed
  • Affects Version/s: 2.5.0
  • Fix Version/s: 2.6.0
  • Component/s: JMock 2.x.x Library
  • Labels:
    None
  • Number of attachments :
    1

Description

Asserting a state-machine's state requires the internal API (not obvious, clear or documented) as in
assertTrue(stateMachine.is("desired-state").isActive());
assertTrue(stateMachine.isNot("undesirable-state").isActive())

I suggest the creation of a become("state") counterpart. Maybe something like isCurrently("state") & isNotCurrently("state") which wrap the internal API, such that we get
assertTrue(stateMachine.isCurrently("state"));
assertTrue(stateMachine.isNotCurrently("state"));

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Steve Freeman added a comment -

How about:

assertThat(stateMachine, isCurrently("state"))

for example?

Show
Steve Freeman added a comment - How about: assertThat(stateMachine, isCurrently("state")) for example?
Hide
Permalink
Nat Pryce added a comment -

What's wrong with

assertStateMachine(banana.is("curved"))

assertStateMachine(banana.isNot("overripe"));

as originally suggested?

That would be both readable and require less implementation effort and duplicated code.

Show
Nat Pryce added a comment - What's wrong with assertStateMachine(banana.is("curved")) assertStateMachine(banana.isNot("overripe")); as originally suggested? That would be both readable and require less implementation effort and duplicated code.
Hide
Permalink
Marcos Sanchez added a comment -

Simplest implementation (I can see):

State is implemented as a String so we

Extend org.jmock.States to include a currentState getter: currently()
Implement in org.jmock.internal.StateMachine

Resulting code allows

assertThat(machine.currently(), is("running"));
assertThat(machine.currently(), is(not("broken")));

and, less elegantly, in Instinct

expect.that(list.currently()).isEqualTo(UNMODIFIED);
expect.that(list.currently()).isNotEqualTo(MODIFIED);

This really needs to be more robust, maybe Enumming possible states and forcing the check against the Enum, but the patch works.

Show
Marcos Sanchez added a comment - Simplest implementation (I can see): State is implemented as a String so we Extend org.jmock.States to include a currentState getter: currently() Implement in org.jmock.internal.StateMachine Resulting code allows assertThat(machine.currently(), is("running")); assertThat(machine.currently(), is(not("broken"))); and, less elegantly, in Instinct expect.that(list.currently()).isEqualTo(UNMODIFIED); expect.that(list.currently()).isNotEqualTo(MODIFIED); This really needs to be more robust, maybe Enumming possible states and forcing the check against the Enum, but the patch works.
Hide
Permalink
Nat Pryce added a comment - - edited

That is not the simplest implementation. It requires an additional getter on States and a new matcher method "is" that is a synonym for "eq" and therefore has a different meaning to the method with the same name in org.hamcrest.Matchers.

The getter also exposes the internal implementation details of the StateMachine class, in particular that it uses null to represent the unnamed initial state.

That violates the "never pass null between objects" design rule.

The assertions using matchers to compare strings also duplicate the implementations of the internal StatePredicate interface.

The API I suggested requires one new assertion method that uses the existing internal API and does not expose internal implementation details of the StateMachine class.

Show
Nat Pryce added a comment - - edited That is not the simplest implementation. It requires an additional getter on States and a new matcher method "is" that is a synonym for "eq" and therefore has a different meaning to the method with the same name in org.hamcrest.Matchers. The getter also exposes the internal implementation details of the StateMachine class, in particular that it uses null to represent the unnamed initial state. That violates the "never pass null between objects" design rule. The assertions using matchers to compare strings also duplicate the implementations of the internal StatePredicate interface. The API I suggested requires one new assertion method that uses the existing internal API and does not expose internal implementation details of the StateMachine class.
Hide
Permalink
Marcos Sanchez added a comment -

Minimal currently() patch as described

Show
Marcos Sanchez added a comment - Minimal currently() patch as described
Hide
Permalink
Marcos Sanchez added a comment -

Ok! Point taken.

So here is your option and a couple of Matchers for good measure!

Supported Syntax includes

assertStateMachine(list.isNot(MODIFIED));
assertStateMachine(list.is(UNMODIFIED));

and

assertThat(list, isCurrently(UNMODIFIED));
assertThat(list, isNotCurrently(MODIFIED));

using

static import org.jmock.Assert;

Show
Marcos Sanchez added a comment - Ok! Point taken. So here is your option and a couple of Matchers for good measure! Supported Syntax includes assertStateMachine(list.isNot(MODIFIED)); assertStateMachine(list.is(UNMODIFIED)); and assertThat(list, isCurrently(UNMODIFIED)); assertThat(list, isNotCurrently(MODIFIED)); using static import org.jmock.Assert;
Hide
Permalink
Nat Pryce added a comment -

I prefer the assertThat style. We can write two matchers that call the internal state machine APIs.

The patch is not acceptable as it stands because:

  • it does not contain any acceptance or unit tests for the new functionality
  • it throws exceptions with no error messages
  • it mixes new functionality and irrelevant formatting changes
  • the matcher descriptions do not fit cleanly into a longer description: they should be a clause of a sentence, not start with a capital letter.
Show
Nat Pryce added a comment - I prefer the assertThat style. We can write two matchers that call the internal state machine APIs. The patch is not acceptable as it stands because:
  • it does not contain any acceptance or unit tests for the new functionality
  • it throws exceptions with no error messages
  • it mixes new functionality and irrelevant formatting changes
  • the matcher descriptions do not fit cleanly into a longer description: they should be a clause of a sentence, not start with a capital letter.
Hide
Permalink
Marcos Sanchez added a comment -

removed assertStateMachine(boolean)

and addressed issues.

Show
Marcos Sanchez added a comment - removed assertStateMachine(boolean) and addressed issues.
Hide
Permalink
Nat Pryce added a comment -

Unfortunately that patch is still not acceptable.

  • Matchers must be stateless (e.g. immutable). Your matcher stores the matched parameter in an instance variable.
  • The patch contains commented out code
  • The patch mixes formatting changes and new functionality.

I've committed an implementation to SVN head.

Show
Nat Pryce added a comment - Unfortunately that patch is still not acceptable.
  • Matchers must be stateless (e.g. immutable). Your matcher stores the matched parameter in an instance variable.
  • The patch contains commented out code
  • The patch mixes formatting changes and new functionality.
I've committed an implementation to SVN head.
Hide
Permalink
Nat Pryce added a comment -

Implementation committed to SVN.

Show
Nat Pryce added a comment - Implementation committed to SVN.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.