Warp: fluent API for request execution - naming

[lfryc]

Warp.execute(ClientAction).verify(ServerAssertion);

There has been two proposals for name change arised recently:

ServerAssertion -> ServerAction

https://community.jboss.org/message/763223#763223

Anthony Patricio wrote:

What disturbs me is the method name "verify" + the interface nameServerAssertion which tend to imply we should not update any serverstate there.

ClientAction -> ClientInvoker

https://community.jboss.org/message/777357#777357

Dan Allen wrote:

Whenever I see the ClientAction, I want to change it in my mind to be ClientInvoker#invoke(). For some reason, that seems more natural to me. I think it's because "action" is so strongly associated with server-side web frameworks that I keep thinking that is server-side code at first (even though it clearly says client). (It so happens that RESTEasy uses ClientInvoker for it's client wrapper too).

The original intentions of naming:

execute(ClientAction) - a given client action leads into reaction of server, which should be verified

verify(ServerAssertion) - a object carrying assertions which should be verified on server

It is now clear that Warp does not server only this original purpose anymore.

Warp helps to execute client's logic in order to issue request against server, where it can execute any server logic and all the exceptions (including assertion failures) will be propagated back to client.

I agree change is important here, though I'm not strong about any of naming:

• execution
• invocation
• triggering action
• verification (this is only against which are strong arguments)
• ...

I'm open to discussion here.

This is especially important in time when Alpha2 is getting closer!

[dan.j.allen]

Lukáš Fryč wrote:

It is now clear that Warp does not server this original purpose anymore.

I definitely still serves its original purpose, it's just way more awesome now! I like the new mission statement.

I also agree that verify() is too narrowly focused, despite the fact that it is one of the key functions it performs. If we take another perspective, a good option emerges.

Warp establishes a causality - a relationship between an event (the cause) and a second event (the effect), where the second event is understood as a consequence of the first. The server-side code (currently verify()) is given an opportunity to observe the effect. In the case of a test, however, it does more than just observe. It's ultimately going to assert something (likely). But just being on the server-side doesn't automatically give it something to assert. It has to weave into the lifecycle to probe and/or query for information. It can then assert (verify) the expected result.

The combination of poking around the server-side lifecycle and expecting certain criteria leads me to the word inspect. Inspect means to view closely in critical appraisal. It usually results in asserting acceptance criteria and raising a red flag if it isn't met.

Thus, I think that verify() should be renamed to inspect() (or inspectServer()) and ServerAssertion should be renamed to ServerInspection.

As for the other method, I'm still leaning towards ClientInvoker. Though, what might work is to just change the action() method to invoke():

public interface ClientActions() {
     public void invoke();
}

Alternative method names: proceed(), do(), play(), perform()

I like the plural form ClientActions since the client (browser) may do several things before you get to a ServerInspection. An alternative name is ClientActivity. I kind of like that one.

[aslak]

Maybe bring it up a level of abstraction? BDD style..

Warp.when(Action).given(Filter).then(Inspect)

[dan.j.allen]

Winning.

[kenfinni]

Was actually thinking the same, I like Aslak's suggestion.

[lfryc]

I think we hit a winner here:

Warp.given(Activity).when(Filter).then(Inspect);

I simply love it.

And a compact naming of interfaces Activity and Inspect is also good proposal - helps to keep the code readable.

[lfryc]

@RunAsClient
@WarpTest
@RunWith(Arquillian.class)
public class TestFocusValidationAware {

    ...

    @Test
    public void testGlobalMessageIsIgnored() {

        Warp
            .given(new Activity() {
                public void perform() {
                    guardHttp(submitButton).click();
                }
            })
            .when(JsfViewFilter.class)
            .then(new Inspect() {

                @BeforePhase(Phase.RENDER_RESPONSE)
                public void addGlobalMessage() {
                    FacesContext context = FacesContext.getCurrentInstance();
                    context.addMessage(null, new FacesMessage("global message"));
                }

                @AfterPhase(Phase.RENDER_RESPONSE)
                public void verifyGlobalMessageIsIgnored() {
                    FacesContext context = FacesContext.getCurrentInstance();

                    AbstractFocus component = bean.getComponent();
                    FocusRendererBase renderer = bean.getRenderer();
                    String candidates = renderer.getFocusCandidatesAsString(context, component);

                    assertEquals("form", candidates);
                }
            });

        assertEquals(input3, getFocusedElement());
    }

    @Test
    public void testGlobalMessageIsIgnored_alternativeSyntax() {

        Warp.given(submitButton).click();
        Warp.when(JsfViewFilter.class);
        Warp.then(new Inspect() {

            @BeforePhase(Phase.RENDER_RESPONSE)
            public void addGlobalMessage() {
                FacesContext context = FacesContext.getCurrentInstance();
                context.addMessage(null, new FacesMessage("global message"));
            }

            @AfterPhase(Phase.RENDER_RESPONSE)
            public void verifyGlobalMessageIsIgnored() {
                FacesContext context = FacesContext.getCurrentInstance();

                AbstractFocus component = bean.getComponent();
                FocusRendererBase renderer = bean.getRenderer();
                String candidates = renderer.getFocusCandidatesAsString(context, component);

                assertEquals("form", candidates);
            }
        });

        assertEquals(input3, getFocusedElement());
    }
}

Message was edited by: Lukáš Fryč

[dan.j.allen]

That looks nice!

One hiccup is that "do" is a reserved keywork. I think that "perform" is a good replacement.

While I like the brevity of Activity and Inspect, I think that where the code is executing gets lost. I'd post a sample with ClientActivity and InspectServer as a comparison and see what feedback we get.

One side note, I'd really like to see the annotations on the class get trimmed down. Three seems like too many. Here are some possible solutions that are functionality equivalent to above:

@RunWith(ArquillianWarp.class) // implies @RunAsClient
public class MyTest { ... }

@RunWith(Arquillian.class)
@WarpTest // implies @RunAsClient

@RunWith(Arquillian.class)
@WarpEnabled // implies @RunAsClient

I like the first and third best. There is no harm in extending the Arquillian.class w/o additional functionality but to serve as an indicator for the extension.

You might think about renaming the Warp API to WarpDrive, WarpExchange or WarpInterchange (I like WarpDrive because it implies activity).

[lfryc]

I was thinking about @RunAsClient implied by @WarpTest too - let's do that: ARQ-1214.

I agree WarpTest isn't as cool as....

@Warped
@RunWith(Arquillian.class)

But I'm not stronly against @RunWith(ArquillianWarp.class)

Dan Allen wrote:

You might think about renaming the Warp API to WarpDrive, WarpExchange or WarpInterchange (I like WarpDrive because it implies activity).

I don't get this change - the only reason would be if we will need Warp for another API purpose.

[dan.j.allen]

Dan Allen wrote:

You might think about renaming the Warp API to WarpDrive, WarpExchange or WarpInterchange (I like WarpDrive because it implies activity).

I don't get this change - the only reason would be if we will need Warp for another API purpose.

Ignore that. I had a bunch of suggestions and at one point I was going to suggest @Warp as the annotation on the test, but changed it.

I like @Warped, but I'm also still keen on @WarpEnabled. Let's see if we get any other input.

• @Warped @RunWith(Arquillian.class)
• @WarpEnabled @RunWith(Arquillian.class)
• @RunWith(Arquillian.class) @EnableWarp
• @RunWith(ArquillianWarp.class)

Warp is a new sort of way to manage the test, so it very well could warrant the custom runner name.

[dan.j.allen]

The value for when() could be improved. I think we need to imply not filtering, but rather observer, weaver or listener.

when(ObservingJsfLifecycle.class)

when(MonitorJsfLifecycle.class)

when(InsideJsfLifecycle.class)

The last one reads the best of the three, IMO.

[lfryc]

when(request().is(JsfResourceRequest.class))

when(request().is(JsfPageRequest.class))

It matches Jakub's proposal for filter builders:

when(request().uri(contains("/rest/service"))

[aslak]

Dan Allen wrote:

@RunWith(ArquillianWarp.class) // implies @RunAsClient
public class MyTest { ... }

-1

This force Warp to care about the TestRunner. It shouldn't.

Dan Allen wrote:

@RunWith(Arquillian.class)
@WarpEnabled // implies @RunAsClient

This should be possible, but not in current impl. Run mode is infered by @Deployment.testable=false or @RunAsClient on class/method level. Warp require testable=true to invoke the Packagaing SPIs, and RunAsClient is read directly from the Metohd / Class.

This features besically needs the same as the alternative language, a metadata layer for the TestClass/TestMethod: https://issues.jboss.org/browse/ARQ-472

[lfryc]

Dan Allen wrote:

While I like the brevity of Activity and Inspect, I think that where the code is executing gets lost. I'd post a sample with ClientActivity and InspectServer as a comparison and see what feedback we get.

Right, one another idea is that we doesn't have to verify just server state, but we might want to intercept request in the middle - e.g. on proxy,:

then(new Inspect() {

     @AfterRequest

      public void verifyRequest(HttpRequest request) {

             assertThat(request.getHeader("Cache-Control"), contains("max-age=3600"));

      }

}

I assume that we can have multiple Inspects then:

InspectServer

InspectRequest

[aslak]

Lukáš Fryč wrote:

@RunAsClient
@WarpTest
@RunWith(Arquillian.class)
public class TestFocusValidationAware {
 
    @Test
    public void testGlobalMessageIsIgnored_alternativeSyntax() {
 
        Warp.given(submitButton).click();
        Warp.when(JsfViewFilter.class);
        Warp.then(new Inspect() {
 
            @BeforePhase(Phase.RENDER_RESPONSE)
            public void addGlobalMessage() {
                FacesContext context = FacesContext.getCurrentInstance();
                context.addMessage(null, new FacesMessage("global message"));
            }
 
            @AfterPhase(Phase.RENDER_RESPONSE)
            public void verifyGlobalMessageIsIgnored() {
                FacesContext context = FacesContext.getCurrentInstance();
 
                AbstractFocus component = bean.getComponent();
                FocusRendererBase renderer = bean.getRenderer();
                String candidates = renderer.getFocusCandidatesAsString(context, component);
 
                assertEquals("form", candidates);
            }
        });
 
        assertEquals(input3, getFocusedElement());
    }
}

Personally i would prefer a chained api as apposed to separate method calls in order.

A chained API describes the allowed order and what can be combined at compile time.

given(X)

  .when(Y) // optional step

   .then(Z) // the chain is broken and return a result

With a non chained API you no longer have compile time verification, this compiles fine but fails runtime:

then(Z) // runtime error, missing given

given(X)

when(Y)

I would also avoid using given() as a Activity builder to keep the Warp API clean and follow it's original design goal of being client library independent. given(driver).click() will bind the Warp API to WebDriver. It's not that much harder to write given(web(button).click()).