I am a firm believer in self-documenting code
so if you see a code and you see the comment; it means something is wrong. Yes, there are exceptions to the rule for example complex business logic. But this story is not an exception from this rule.
One of the critical aspects of the self-documenting code is naming. Proper method names can save a lot of time. Of course, it is not easy.
I am far away from a master of naming myself, and I am still learning, so today we will study few bed names.
Name vs Comment
[csharp]
public override User GetSomeUsers()
{
//some code<img src="https://wyrodek.pl/wp-content/uploads/2017/11/asciiflower6-300×293.png" alt="" width="300" height="293" class="aligncenter size-medium wp-image-297" />
/ /get new response from api
return StoreObjectHelper.RetriveUserFromUsersDB(userNumber).FirstorDefault(); // it retrieves it from api, not database
}
[/csharp]
We live in the era of IDE that can do quite a lot. And Name refactoring is one of the things it can do. [expand]yes you have to be careful sometimes you may touch something you don’t want to. [/expand]
But in this case, There is no reason not to rename it. After checking we find out that API is called CustomerAPI. In that case, we can call our method
GetUserFromCustomerAPI.
[csharp]
public override User GetSomeUsers()
{
//some code
/ /get new response from api
return StoreApiHelper.GetUserFromCustomerAPI(userNumber).FirstorDefault(); // it retrieves it from api, not database
}
[/csharp]
Yes behind the scenes I’ve also split helper so we can put it in something more useful.
That was Fast, but that was an appetizer. The main dish is coming right this way!
Mea OF The Day Confusing Names
[csharp]
public bool VerifyViewXMLCloseButton()
{
if (!ViewOrderUXPage.ViewXMLCloseButton.Displayed)
{
return false;
}
ViewOrderUXPage.ViewXMLCloseButton.Click();
return driver.Url.Contains(Helpers.AsnUxDetailPage);
}
[/csharp]
Well, first those names are Difficult to read We will address it on the way.
Second, you want to Verify if close button is displayed. And it is closing page.
Plus I assume it is validating if a page was closed properly.
Ergo it is doing THREE things. This is breaking single responsibility principle.
[csharp]
public class ViewXMLModal : PageBase
{
//other funtions
public IWebElement CloseButton { get; set; }
public bool IsCloseButtonPresent()
{
return CloseButton.Displayed;
}
public void CloseModal()
{
if (CloseButton.Displayed)
{
CloseButton.Click();
}
}
public bool IsModalClosed()
{
return driver.Url.Contains(Helpers.AsnUxDetailPage);
}
}
[/csharp]
Above code is breaking a rule of refactoring I am changing how the function operates here. In reality, this is effect of folwing steps
1. Move modal action to new page object for modal.
2. Run affected the test.
3. Split methods away.
4. Fixed tests.
Why am I using IsCloseButtonPresent method in the CloseModal? Well, Author cared for it not to throw an exception so we should do the same. The only problem is that that also changes the actual way how assertion worked. We will talk about it in a moment.
We moved modal action away to different object now we can refer to them in form viewXmlModal.IsCloseButtonPresent it is more readable then IsViewXMLModalCloseButtonPresent
Ok all fine but we have we did change logic. Now we have to change test that is using it. I could do it cause I had only one test to edit. But What to do in the case when you have more like 40?
As band-aid but you could rename it for something more meaning full and then tackle it as tech debt story when you have more time.
CloseModalAndVerifyClosure. – Brrr, I hate this name. But at least it tells you what happens.
Are we done?
No, we are not. You had to notice it was
Cause it is modal. Probably now some of you are confused „why is modal changing page URL?”
Answer it is not. This assertion will always be true. I won’t fix it here. But next time we will talk assertions, and we will start with it.
Before we wrap up remember Naming is important.
This post is part of larger series on antipatterns in testing you can check it here.