Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix form validation #646

Merged
merged 3 commits into from
Feb 22, 2021
Merged

Fix form validation #646

merged 3 commits into from
Feb 22, 2021

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Feb 19, 2021

FormValidationTest has been reliably failing for a while.

This fixes the cause of failure where the validation being selected was not the correct one.
possibly caused by jenkinsci/jenkins#5195

Given this is in the upcoming LTS and mainline I have not bothered about backwards compatibility.

e.g. https://ci.jenkins.io/job/Core/job/acceptance-test-harness/job/master/lastCompletedBuild/testReport/core/FormValidationTest/java_8_split9___validate/

@jtnord jtnord requested review from Vlatombe, fbelzunc, timja, bmunozm and fqueiruga and removed request for fbelzunc February 19, 2021 15:17
FormValidation was failing as the structure of the validation changed.

```
Too many validation elements: [[[[[[[FirefoxDriver: firefox on LINUX
(5967b81a-5dce-41f8-a115-117b72e60140)] -> css selector:
[path='/useincluderegex/includeRegex']]] -> xpath:
./../../following-sibling::tr/td[2] | ./../following-sibling::div]] ->
tag name: div], [[[[[[FirefoxDriver: firefox on LINUX
(5967b81a-5dce-41f8-a115-117b72e60140)] -> css selector:
[path='/useincluderegex/includeRegex']]] -> xpath:
./../../following-sibling::tr/td[2] | ./../following-sibling::div]] ->
tag name: div], [[[[[[FirefoxDriver: firefox on LINUX
(5967b81a-5dce-41f8-a115-117b72e60140)] -> css selector:
[path='/useincluderegex/includeRegex']]] -> xpath:
./../../following-sibling::tr/td[2] | ./../following-sibling::div]] ->
tag name: div], [[[[[[FirefoxDriver: firefox on LINUX
(5967b81a-5dce-41f8-a115-117b72e60140)] -> css selector:
[path='/useincluderegex/includeRegex']]] -> xpath:
./../../following-sibling::tr/td[2] | ./../following-sibling::div]] ->
tag name: div]]
```

THis fixes the FormValidationTest (to some extent) however there is a
bug in firefox that causes it not to show the alert when navigating away
from a configuration page.  works with chrome so this is at least a step
in the right direction.
@jtnord jtnord force-pushed the fix-form-validation branch from def3c3c to 21408b6 Compare February 19, 2021 15:20
@jtnord jtnord added the fix label Feb 19, 2021
…java

Co-authored-by: Vincent Latombe <vincent@latombe.net>
@MRamonLeon
Copy link
Contributor

MRamonLeon commented Feb 19, 2021

optional reviewer: @raul-arabaolaza FYI

@@ -67,23 +67,24 @@ public static FormValidation await(Control control) {

// Special handling for validation buttons and their markup
if (element.getTagName().equals("button")) {
WebElement spinner = element.findElement(control.by.xpath("./../../../following-sibling::div[1]"));
WebElement spinner = element.findElement(by.xpath("./../../../following-sibling::div[1]"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was control affecting by any means?

Copy link
Member Author

@jtnord jtnord Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/jenkinsci/acceptance-test-harness/pull/646/files#diff-6272a530cea8f96e9ff459b762ff966c5a25650cfaf3229431b06bec54dfd269R37

by is a static field of CapybaraPortingLayer of which Control extends, So this is unrelated to the fix - just removing the "Field by of blah should be accessed in a static way" linting.

Copy link
Contributor

@MRamonLeon MRamonLeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤞🏽 looking forward to seeing the test green

@Vlatombe Vlatombe merged commit ed49220 into jenkinsci:master Feb 22, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants