Appian needs to start using good coding practices with their Cucumber for Appian automation framework.

jesric1029 over 1 year ago

I've raised these issues to Appian in a support email, and no response was returned, and I have not seen any improvement in the problem in the year since. I've spotted two major issues with the Appian automation framework, which are just lousy coding and must be resolved.

Problem 1: The Tear Down Method

Currently Appian introduces the teardown method as a step in your feature file. This is a problem for two reasons.

1. Ending the test execution early will never reach the teardown method.

2. A test that fails before it reaches the teardown method will not be adequately torn down.

Impact:

When you do not tear down the Chrome or Ghecko driver, you have two major issues, one minor annoyance and the other more significant. First, the browser will not close. If you run the test ten times, you will have ten browsers open. This alone may not be a big deal since you can close them, but the bigger problem is that the Chrome driver instance will still be alive in your processes and slowly eat away at your memory. Imagine running a test twenty times and ending the execution early. You will have twenty drivers running in your PC or Mac processes. Unless you know what you're looking for, many people will have no idea about this, and their PC or Mac will go slower until it must be restarted.

Solution:

This is the same solution I implemented on my own, which I have provided to Appian previously. The teardown method should be called a Junit @AfterClass method so that it runs at the end of your test regardless of whether it passes or fails. Now there is one exception to this rule - if you use the examples section in your feature file, you do not want to use this solution and will just have to be sure to kill the background process.

@AfterClass
public static void tearDown() throws IOException {
    fixture.tearDown();

See a very easy solution!

Problem 2: Appian is using very poor coding practices in the TempoFixture class of the jar and creating unnecessary objects. This is basic Java coding 101. Source.

Here is a perfect example of Appian breaking this basic coding rule:

public void clickOnAction(String actionName) {
    TempoAction.getInstance(this.settings).waitFor(new String[]{actionName});
    TempoAction.getInstance(this.settings).click(new String[]{actionName});
}

I am a former software developer, so I understand how a jar works and precisely what is in the code before I start trying to work with it. I do this partly to determine if the jar meets Java quality standards or will slow down my automation for no good reason. If you are not a Java expert, let me highlight precisely what they are doing wrong here.

1. This is a void method. That means that this method only clicks on the action and will never be used again.

2. In this example, Appian has created two completely unnecessary objects. We only have to look at the code below to understand where:

.click(new String[]{actionName});

.waitFor(new String[]{actionName});

3. We are already passing in the action name at the beginning of the method: (String actionName.) There is no reason to create a strong array object called "actionName" that contains precisely the same value we already passed in. This method is clicking on one action, and that object will never be used again. So we have wasted processing time by creating an unnecessary object which will waste speed.

4. To demonstrate how bad of a coding practice this is, imagine if you created a loop to click on the same action repeatedly. This would mean we would be creating an infinite number of objects until, at some point, Java crashes.

Impact: This reduces the speed of clicking on the action. This might be a very small impact, but with many different tempo actions taking place, those small-time increases add up over time.

Solution: The solution is quite simple:

public void clickOnAction(String actionName) {
    TempoAction.getInstance(this.settings).waitFor(actionName);
    TempoAction.getInstance(this.settings).click(actionName);
}

Now we are not creating an object to click on an action but just using the value passed into the method to click on it. Based on my review of the jar class, I would estimate that Appian must have seen they had at least 100 warnings when committing their code but ignored them. I have no idea how many similar issues there are in other classes. I am hoping this will get the attention of Appian and they can resolve these bad practices.

Discussion posts and replies are publicly visible