Способы улучшения этого кода

Question

Я пытаюсь написать некоторый тестовый код для моего приложения Java с помощью Scalatest. Я понял, поскольку Scala имеет гораздо более читаемый синтаксис, это приведет к появлению более читаемого тестового кода.

До сих пор, это то, что мне удалось:

 
package com.xyz 

import org.scalatest.FlatSpec 
import org.scalatest.matchers.ShouldMatchers 
import com.xyz.SecurityService 
import org.mockito.Mockito._ 
import org.scalatest.mock.MockitoSugar 
import org.mockito.Matchers._ 
import javax.servlet.jsp.tagext.Tag 

class CheckRoleTagSpec extends FlatSpec with ShouldMatchers with MockitoSugar { 

    behavior of "CheckRole tag" 

    it should "allow access when neither role nor root defined" in { 
    val securityServiceMock = mock[SecurityService] 

    val tag = new CheckRoleTag() 
    tag.setSecurityService(securityServiceMock) 

    tag.setGroup("group") 
    tag.setPortal("portal") 

    tag.setRoot(false) 
    tag.setRole(null) 

    tag.doStartTag should be(Tag.SKIP_BODY) 
    } 

} 

Я очень разочарован с этим кодом. Это практически то же самое, что мне нужно писать на Java. Пожалуйста, помогите мне сделать его более scala-like и функциональным.

Answer 1

Приведенный ниже код создает новый анонимный класс, но doStartTag возвращает результат, как и ожидалось:

... 
(new CheckRoleTag{ 
    setSecurityService(mock[SecurityService]) 
    setGroup("group") 
    setPortal("portal") 
    setRoot(false) 
    setRole(null) 
} doStartTag) should be(Tag.SKIP_BODY) 
... 

Answer 2

Поскольку этот конкретный тест просто вызывает кучу сеттеров на объект реализован в Java, есть не много вы может сделать его более кратким или функциональным или скалистым. Вы можете удалить некоторые повторения с чем-то вроде

it should "allow access when neither role nor root defined" in { 
    val securityServiceMock = mock[SecurityService] 

    val tag = new CheckRoleTag() 

    locally { 
    import tag._ 
    setSecurityService(securityServiceMock) 
    setGroup("group") 
    setPortal("portal") 
    setRoot(false) 
    setRole(null) 
    } 

    tag.doStartTag should be(Tag.SKIP_BODY) 
} 

Я не уверен, является ли это на самом деле стоит в этом случае.

Answer 3

Вы не можете исправить уродливое испытание, переписав тест. Вы можете исправить это, изменив API, который тестируется.

Ну, технически, это является можно писать уродливые тесты для хороших API, если вы пытаетесь действительно трудно, очень злой, очень, глупый, очень пьян или очень устал. Но написание уродливого теста требует усилий, а программисты ленивы, поэтому маловероятно, чтобы кто-то написал по ошибке уродливый тест. Практически невозможно писать уродливые тесты: вы что-то вставляете, вы получаете что-то, вы проверяете, получили ли вы то, что ожидали. Вот и все. Там действительно нечего угадывать.

Тест просто использует API так же, как пользователь API будет использовать его. Это в основном пример того, как правильно использовать API, что также почти как побочный эффект, чтобы проверить, действительно ли API на самом деле правильно реализован. Вот почему уродливый тест является хорошим показателем плохого дизайна API, и именно поэтому тест-драйв дизайна API - это хорошо, даже если вы не делаете TDD.

В этом конкретном случае я вижу несколько способов улучшить API, хотя эти предложения обязательно неполные, мелкие и упрощенные (не говоря уже о том, что они ошибочны), поскольку я ничего не знаю о вашем домене:

• Better Имена: setRoot звучит, как он устанавливает корень. Но, если false не является корнем вашей иерархии, я предполагаю, что то, что это Фактически параметр является ли этот тег корнем. Поэтому его следует называть isRoot или makeRoot или setIsRoot или что-то в этом роде.
• Better Defaults: продолжение с setRoot, считая, что моя догадка правильная, и устанавливает, является ли тег корнем, а по умолчанию неправильный путь. По самому определению понятия «корень», может быть только один корень.Таким образом, вы заставляете своих пользователей указывать setRoot(false)каждый раз, за исключением этого время, когда они фактически определяют корень. Не-корневые теги должны быть по умолчанию, и вы должны быть принуждены к setRoot(true) за это один Тег, который на самом деле является корень.
• Улучшения по умолчанию, часть II: setRole(null). Шутки в сторону? Вы заставляете своих пользователей явно указывать роль unset? Почему бы просто не отключить настройку по умолчанию? В конце концов, тест называется «... когда ни роль, ни корень не определены», так зачем их определять?
• Fluent API/Builder Pattern: если вы действительно должны построить недействительные объекты (но смотри следующий пункт), по крайней мере, использовать что-то вроде Fluent API или Builder шаблон.
• Only Construct Valid Objects: На самом деле объекты всегда должны быть действительными, полными и полностью сконфигурированными при построении. Вам не нужно будет создавать объект, а затем настройте его.

Таким образом, тест в основном становится:

package com.xyz 

import org.scalatest.FlatSpec 
import org.scalatest.matchers.ShouldMatchers 
import com.xyz.SecurityService 
import org.mockito.Mockito._ 
import org.scalatest.mock.MockitoSugar 
import org.mockito.Matchers._ 
import javax.servlet.jsp.tagext.Tag 

class CheckRoleTagSpec extends FlatSpec with ShouldMatchers with MockitoSugar { 
    behavior of "CheckRole tag" 
    it should "allow access when neither role nor root defined" in { 
    val tag = new CheckRoleTag(mock[SecurityService], "group", "portal") 

    tag.doStartTag should be(Tag.SKIP_BODY) 
    } 
}