4911-refactoringRadioButtonTests #5087
Conversation
MayaElf
changed the title
MayaElf
force-pushed
the
4911_radioButtonTestsRefactoring
branch
from
b7e3ec3 to
b1559df
Compare
MayaElf
changed the title
| @@ -17,15 +17,15 @@ public class RadioButtons extends com.epam.jdi.light.ui.html.elements.complex.Ra | |||
| @Override | |||
| public WebList list() { | |||
| WebList radioBtnWebList = new WebList(base()).setup(b -> b.setSearchRule("Any", ANY_ELEMENT)) | |||
| .setUIElementName(LABEL); | |||
| .setUIElementName(LABEL); | |||
There was a problem hiding this comment.
мы не трогаем никакой проект, кроме ангуляра
|
|
||
| /** | ||
| * To see an example of RadioButtons web element please visit https://material.angular.io/components/radio/overview. | ||
| */ | ||
|
|
||
| public class RadioButtons extends UIBaseElement<RadioButtonsAssert> { | ||
| public class RadioButtons extends UIListBase<UISelectAssert<RadioButtonsAssert, RadioButtons>> { |
There was a problem hiding this comment.
Согласно наименованиям angular это называется RadioGroup, мы всегда сохраняем имена
RadioButton - отдельный элемент со своими свойствами, именно их список мы должны уметь возвращать
|
|
||
| @JDIAction("Click '{name}' button by tag value '{0}'") | ||
| public void click(String value) { | ||
| getRadioButtonByTagValue(value).click(); | ||
| getRadioButtonByTagValue(value).find(By.cssSelector(".mdc-form-field")).click(); | ||
| } | ||
|
|
||
| @JDIAction("'{name}' radio button contain value '{0}'") |
There was a problem hiding this comment.
комментарий не соответствует возвращаемому значению
jdi-light-angular/src/main/java/com/epam/jdi/light/angular/elements/complex/RadioButtons.java
Outdated
Show resolved
Hide resolved
| @@ -32,6 +35,35 @@ private UIElement getRadioButtonByTagValue(String value) { | |||
| return element; | |||
| } | |||
|
|
|||
| public AngularColors color(String value) { | |||
There was a problem hiding this comment.
цвет - свойство отдельного элемента, вот туда и надо его вынести
There was a problem hiding this comment.
у нас в других реализациях (смотрела bootstrap и material-ui) тоже сделано только через группу, нет отдельного элемента кнопки.
вы предлагаете в ангуляре сделать отдельную реализацию для кнопки, правильно понимаю комментарий?
| if (getRadioButtonByTagValue(value).hasClass("mat-primary")) { | ||
| return AngularColors.PRIMARY; | ||
| } | ||
| if (getRadioButtonByTagValue(value).hasClass("mat-warn")) { |
There was a problem hiding this comment.
каждый раз вызывать поиск элемента - очень плохо, элемент нужно получать один раз
|
|
||
| @JDIAction("'{name}'aria-label is '{0}'") | ||
| public boolean isAriaLabel(String ariaLabel) { | ||
| return core().attr("aria-label").equalsIgnoreCase(ariaLabel); |
There was a problem hiding this comment.
тоже самое с area-label, как в предыдущем ПР
|
|
||
| @JDIAction("'{name}'label is in before position") | ||
| public boolean isBeforePosition() { | ||
| return core().attr("labelposition").equalsIgnoreCase("before"); |
There was a problem hiding this comment.
свойство может стоять на группе (или не стоять) и отдельно у каждого radio может быть переопределено, это половина решения, получается
MayaElf
force-pushed
the
4911_radioButtonTestsRefactoring
branch
from
de3722f to
a25a035
Compare
| return core().hasAttribute("disabled"); | ||
| } | ||
|
|
||
| @JDIAction("'{name}'is required") |
There was a problem hiding this comment.
| @JDIAction("'{name}'is required") | |
| @JDIAction("'{name}' is required") |
| return getRadioButtonByTagValue(value).attr("class").contains("mat-mdc-radio-checked"); | ||
| } | ||
|
|
||
| private UIElement getRadioButtonByTagValue(String value) { |
There was a problem hiding this comment.
Метод должен быть public, так же должен быть метод, который получает весь список RadioButton.
Допустим, нам нужно сделать проверки для всех элементов в RadioGroup, мы должны иметь возможность получить список и прогнать. Если мы каждый раз будем получать по имени, это будет крайне медленно.
Помимо того, что сейчас у нас нет возможности проверить, какие опции нам туда подсунули.
Я в предыдущем ревью написала, что нам нужен класс для описания группы и для описания каждого отдельного RadioButton
There was a problem hiding this comment.
- public + аннтоция проставлены
- у нас есть реализованный метод getRadioButtons(), как раз используем его для получения кнопки по имени.
@JDIAction("Get radio button by tag value {0}")
public UIElement getRadioButtonByTagValue(String value) {
UIElement element = null;
for (UIElement e : getRadioButtons()) {
if (e.find("input").attr("value").equalsIgnoreCase(value)) {
element = e;
}
}
return element;
} - отдельный RadioButton, насколько я поняла, обсуждаем сегодня 26 10 23 в 16.00 на синке
| import com.epam.jdi.light.common.JDIAction; | ||
| import org.hamcrest.Matchers; | ||
|
|
||
| import static com.epam.jdi.light.asserts.core.SoftAssert.jdiAssert; | ||
|
|
||
| public class RadioButtonsAssert extends UIAssert<RadioButtonsAssert, RadioButtons> { | ||
| public class RadioButtonsAssert extends UISelectAssert<RadioButtonsAssert, RadioGroup> { |
There was a problem hiding this comment.
Раз мы переименовали в RadioButton, и этот класс тоже должен был следом переименоваться
| public class RadioButtonTests extends TestsInit { | ||
| private static final String SPRING = "Spring"; | ||
| private static final String SUMMER = "Summer"; | ||
| private static final String AUTUMN = "Autumn"; | ||
| private static final String WINTER = "Winter"; | ||
| private static final String primary = "primary"; | ||
| private static final String accent = "accent"; | ||
| private static final String warn = "warn"; | ||
|
|
||
| @BeforeMethod |
There was a problem hiding this comment.
В предыдущем ПР уже писала про BeforeMethod
| getRadioButtonByTagValue(value).find(By.cssSelector(".mdc-form-field")).click(); | ||
| } | ||
|
|
||
| @JDIAction("'{name}' radio button contain value '{0}'") |
There was a problem hiding this comment.
комментарий не соответствует функции
MayaElf
force-pushed
the
4911_radioButtonTestsRefactoring
branch
from
1402f58 to
609725d
Compare
| import org.openqa.selenium.By; | ||
|
|
||
| public class RadioButton extends UIBaseElement<RadioButtonAssert> implements HasLabel { | ||
| private static final String INPUT_SELECTION_CONTROL = "//input[@type='radio']"; |
There was a problem hiding this comment.
| private static final String INPUT_SELECTION_CONTROL = "//input[@type='radio']"; | |
| private static final String INPUT_SELECTION_CONTROL = ".//input[@type='radio']"; |
| } | ||
|
|
||
| @JDIAction("'{name}' element label is in before position") | ||
| public boolean isRadioButtonBeforePosition() { |
There was a problem hiding this comment.
| public boolean isRadioButtonBeforePosition() { | |
| public boolean hasBeforePosition() { |
|
|
||
| @JDIAction("'{name}' element label is in before position") | ||
| public boolean isRadioButtonBeforePosition() { | ||
| return attr("labelposition").equalsIgnoreCase("before"); |
There was a problem hiding this comment.
Метод даст неправильный результат для "Radio group label position before 1" и "Radio group label position before 2" с тестового сайта
| if (hasClass("mat-warn")) { | ||
| return AngularColors.WARN; | ||
| } else | ||
| return AngularColors.ACCENT; |
There was a problem hiding this comment.
Для отдельной кнопки если значение не задано в нем самом, оно определено для всей группы
There was a problem hiding this comment.
пересмотрела все наши реализации, цвета на уровне группы нет ни в одной. поэтому, извините, не соображу, как взять цвет с уровня группы
There was a problem hiding this comment.
Ну вот на тестовом сайте вверху есть "Radio buttons configuration", у него только один контрол цветной, остальные темные, а по этому алгоритму получается, что у всех всегда одинаковый код, значит что-то не так в этом алгоритме
|
|
||
| @JDIAction("Click '{name}' radio button with value '{0}'") | ||
| public void click(String value) { | ||
| getRadioButtonByValue(value).find(By.cssSelector(".mdc-form-field")).click(); |
There was a problem hiding this comment.
а почему мы просто не можем по кнопке и кликнуть?
There was a problem hiding this comment.
когда в вертикальной группе кнопка большого размера (как у нас на сайте ), он кликает в середину и не происходит выделения радио батон. клик есть, а выделения нет.
There was a problem hiding this comment.
так у нас клик переопределен, как такое может быть?
There was a problem hiding this comment.
Долго разбиралась, если честно, почему падают тесты. Думала, что это из-за ngModel реализации. А потом перебором вариантов нашла, что лефтэреа клик выделяет. Добавила указание на форм-филд тесты перестали падать. Предполагаю, что даже при переопределении вертикальные кнопки оказываются слишком большие. Если убрать find(By.cssSelector(".mdc-form-field")) , то все тесты на вертикальные радио кнопки падают, клик есть, выделения нет :( Для горизонтальных кнопок все работает без find(By.cssSelector(".mdc-form-field"))
There was a problem hiding this comment.
1 Кликать по mdc-form-field не безопасно, не все части приводят к выделению, надо кликать по mdc-radio
2 Любая такая проблема не повод оставлять непонятный код внутри другого класса
| } | ||
|
|
||
| @JDIAction("'{name}' radio button contain value '{0}'") | ||
| public boolean isChecked(String value) { |
There was a problem hiding this comment.
- почему такого метода нет у RadioButton? это же его информация
- где метод для получения выбранного?
There was a problem hiding this comment.
а почему не используется?
There was a problem hiding this comment.
- по checked
@test(description = "Test verifies value and the radio-button is checked")
public void basicRadioButtonsTest() {
basicRadioGroup.show();
basicRadioGroup.is().displayed();
basicRadioGroup.click("2");
basicRadioGroup.click("1");
basicRadioGroup.click("2");
basicRadioGroup.is().checked("2");
basicRadioGroup.is().notChecked("1");
basicRadioGroup.radioButtons().get(1).is().checked();
basicRadioGroup.radioButtons().get(0).is().notChecked();
} - получение выбранного
@test(description = "Test verifies that there is checked radio-button in the group")
public void getCheckedRadioButtonTest() {
colorRadioGroup.has().checkedRadioButton();
requiredRadioGroup.has().noCheckedRadioButton();
}
There was a problem hiding this comment.
- Еще раз перечитай первый вопрос
- Посмотри на вот эту строку кода https://github.com/jdi-testing/jdi-light/pull/5087/files#diff-278ebeb52c9d2a302e5401e2a7c09a8bbdfea63f44f2c823a1c391e1b0ef7ccdR60
- Исправь ошибку
| public RadioButton getRadioButtonByValue(String value) { | ||
| RadioButton radioButton = null; | ||
| for (RadioButton e : radioButtons()) { | ||
| if (e.find("input").attr("value").equalsIgnoreCase(value)) { |
There was a problem hiding this comment.
а почему у RadioButton нет метода по получению его подписи?
There was a problem hiding this comment.
а почему не используется?
There was a problem hiding this comment.
@test(description = "Test verifies radio button label")
public void radioButtonLabelTest() {
requiredRadioGroup.radioButtons().get(0).has().label("One");
}
There was a problem hiding this comment.
Речь про полное нарушение инкапсуляции, ты залазишь в дочерний элемент, что-то оттуда берешь и считаешь, что это нормально.
Мы можем захотеть работать с элементами по label, можем по value.
config/checkstyle/checkstyle.xml
Outdated
| @@ -5,6 +5,10 @@ | |||
| <module name="SuppressionFilter"> | |||
| <property name="file" value="${samedir}/suppressions.xml" /> | |||
| </module> | |||
| <module name="NewlineAtEndOfFile"> | |||
| <property name="severity" value="error"/> | |||
| <property name="lineSeparator" value="lf_cr_crlf"/> | |||
There was a problem hiding this comment.
этот стиль используется в нескольких проектах, если мы его изменим тут, оно потом может сломать после вливания, ПР на стиль должен быть отдельным ПР в master вместе с исправлением всех файлов, которые этому условию не удовлетворяют
There was a problem hiding this comment.
спасибо, откатила изменения в этом пиаре, сделала отдельный
| if (hasClass("mat-warn")) { | ||
| return AngularColors.WARN; | ||
| } else | ||
| return AngularColors.ACCENT; |
There was a problem hiding this comment.
Ну вот на тестовом сайте вверху есть "Radio buttons configuration", у него только один контрол цветной, остальные темные, а по этому алгоритму получается, что у всех всегда одинаковый код, значит что-то не так в этом алгоритме
|
|
||
| @JDIAction("Click '{name}' radio button with value '{0}'") | ||
| public void click(String value) { | ||
| getRadioButtonByValue(value).find(By.cssSelector(".mdc-form-field")).click(); |
There was a problem hiding this comment.
так у нас клик переопределен, как такое может быть?
| } | ||
|
|
||
| @JDIAction("'{name}' radio button contain value '{0}'") | ||
| public boolean isChecked(String value) { |
There was a problem hiding this comment.
а почему не используется?
| public RadioButton getRadioButtonByValue(String value) { | ||
| RadioButton radioButton = null; | ||
| for (RadioButton e : radioButtons()) { | ||
| if (e.find("input").attr("value").equalsIgnoreCase(value)) { |
There was a problem hiding this comment.
а почему не используется?
| public RadioButton getCheckedRadioButton() { | ||
| RadioButton radioButton = null; | ||
| for (RadioButton e : radioButtons()) { | ||
| if (e.hasClass("mat-mdc-radio-checked")) { |
There was a problem hiding this comment.
у кнопки есть метод, который отдает значение, выделен он или нет, почему не использовать его?
|
|
||
| @JDIAction("Click '{name}' radio button") | ||
| public void click() { | ||
| find(By.cssSelector(".mdc-form-field")).click(); |
There was a problem hiding this comment.
локатор частично дублируется с 42 строкой
MayaElf
force-pushed
the
4911_radioButtonTestsRefactoring
branch
2 times, most recently
from
bb2454f to
0a147ac
Compare
MayaElf
force-pushed
the
4911_radioButtonTestsRefactoring
branch
3 times, most recently
from
b6e5d92 to
2782704
Compare
MayaElf
force-pushed
the
4911_radioButtonTestsRefactoring
branch
from
2782704 to
49c0ac7
Compare
|
|
||
| @JDIAction("Click '{name}' radio button with value '{0}'") | ||
| public void click(String value) { | ||
| getRadioButtonByValue(value).find(By.cssSelector(".mdc-form-field")).click(); |
There was a problem hiding this comment.
1 Кликать по mdc-form-field не безопасно, не все части приводят к выделению, надо кликать по mdc-radio
2 Любая такая проблема не повод оставлять непонятный код внутри другого класса
| } | ||
|
|
||
| @JDIAction("'{name}' radio button contain value '{0}'") | ||
| public boolean isChecked(String value) { |
| public RadioButton getRadioButtonByValue(String value) { | ||
| RadioButton radioButton = null; | ||
| for (RadioButton e : radioButtons()) { | ||
| if (e.find("input").attr("value").equalsIgnoreCase(value)) { |
| RadioButton radioButton = null; | ||
| for (RadioButton e : radioButtons()) { | ||
| if (e.isChecked()) { | ||
| radioButton = e; |
There was a problem hiding this comment.
а почему мы отсюда не возвращаем значение?
There was a problem hiding this comment.
45 строка вернет либо чекнутую радиобатон, либо нулл. оба случая покрыты
@JDIAction("Get '{name}' checked radio-button")
public RadioButton getCheckedRadioButton() {
RadioButton radioButton = null;
for (RadioButton e : radioButtons()) {
if (e.isChecked()) {
radioButton = e;
}
}
return radioButton;
}
There was a problem hiding this comment.
У тебя есть RadioGroup из 50 пунктов, ты переберешь их все, потом только что-то вернешь, хотя знаешь, что выделенным элементом может быть только один. Как исправить проблему длительного выполнения?
| } | ||
|
|
||
| @JDIAction("'{name}' radio button contain value '{0}'") | ||
| public boolean isChecked(String value) { |
There was a problem hiding this comment.
- Еще раз перечитай первый вопрос
- Посмотри на вот эту строку кода https://github.com/jdi-testing/jdi-light/pull/5087/files#diff-278ebeb52c9d2a302e5401e2a7c09a8bbdfea63f44f2c823a1c391e1b0ef7ccdR60
- Исправь ошибку
| RadioButton radioButton = null; | ||
| for (RadioButton e : radioButtons()) { | ||
| if (e.isChecked()) { | ||
| radioButton = e; |
There was a problem hiding this comment.
У тебя есть RadioGroup из 50 пунктов, ты переберешь их все, потом только что-то вернешь, хотя знаешь, что выделенным элементом может быть только один. Как исправить проблему длительного выполнения?
| public RadioButton getRadioButtonByValue(String value) { | ||
| RadioButton radioButton = null; | ||
| for (RadioButton e : radioButtons()) { | ||
| if (e.find("input").attr("value").equalsIgnoreCase(value)) { |
There was a problem hiding this comment.
Речь про полное нарушение инкапсуляции, ты залазишь в дочерний элемент, что-то оттуда берешь и считаешь, что это нормально.
Мы можем захотеть работать с элементами по label, можем по value.
| @JDIAction("'{name}' has no checked radio button") | ||
| public RadioGroupAssert noCheckedRadioButton() { | ||
| jdiAssert(element().getCheckedRadioButton(), Matchers.nullValue(), | ||
| "There is checked radio button in the group"); |
There was a problem hiding this comment.
Ну правильнее написать, что именно выделено, а не просто что что-то выделено, отлаживать же надо, что было
|
|
||
| @Override | ||
| @JDIAction("Get if '{name}' is disabled") | ||
| public boolean isDisabled() { |
There was a problem hiding this comment.
Если мы переопределили isDisabled, то логично и isEnabled переопределить, чтобы быстрее работал
| } | ||
|
|
||
| @JDIAction("'{name}' is disabled") | ||
| public boolean isDisabled() { |
There was a problem hiding this comment.
Тоже должна быть аннотация Override и надо переопределить isEnabled
MayaElf
force-pushed
the
4911_radioButtonTestsRefactoring
branch
from
0eaddfa to
86d62dd
Compare
|
исправления зашли в тупик, все исправила сама |
No description provided.