Skip to content

Fix experimental UI Build With Parameters ATHs #2561

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Draft
RanjitCS wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Fix experimental UI Build With Parameters ATHs #2561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
01a86e9
Fix ATH errors with experimental build-with-parameters
RanjitCS Dec 18, 2025
173380b
Add conditional check on clickLink
RanjitCS Dec 18, 2025
9cfd73b
Use correct flag
RanjitCS Dec 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/main/java/org/jenkinsci/test/acceptance/po/BuildWithParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public BuildWithParameters enter(List<Parameter> definitions, Map<String, ?> val
}

public void start() {
clickButton("Build");
if (Boolean.getBoolean("new-job-page.flag.defaultValue")) {
Copy link
Member

@alecharp alecharp Dec 19, 2025

Choose a reason for hiding this comment

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

My fear is that this is ok in testing while the "original" value of the property is false (when the flag value is coming from the default flag value, coming from the BooleanExperimentalFlag class). But when we change a flag default value (from the flag class itself) from false to true, this won't report the correct status of the flag.

This should be good enough for now but in mid-long term, we would need to have some sort of API in ATH to get the flag value.

cc @jtnord @janfaracik

Copy link
Member

@jtnord jtnord Dec 19, 2025

Choose a reason for hiding this comment

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

This should be good enough for now but in mid-long term, we would need to have some sort of API in ATH to get the flag value.

The value is per user so you would need to do this prior to every request prior to interacting.
It would be better if you can detect this from the page by the page doing something. or just assuming its false and click the button and then try the fallback (new way).

Anyway - this is another case of DO NOT DO THIS AT ALL... use a data-test-id in the actual page for the button in both legacy and new cases - use the ID to get the button. no funny code changes. IE fix upstream.

Copy link
Member

@jtnord jtnord Dec 19, 2025

Choose a reason for hiding this comment

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

the above comment is mostly moot as the build with parameters is not a page in the new UI so this code should be deprecated and the code put into the Build pageObject.

find(by.xpath("//dialog[@open]//div[@id='bottom-sticker']//button[contains(., 'Build')]"))
Copy link
Member

@jtnord jtnord Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

BuildWithParameters is a dialog in the Job page now - so this whole class should probably be deprecated. The Page /build is still present and directly accessible and uses the old format and anyone using this PO to access that page should still work.

.click();
} else {
clickButton("Build");
}
}
}
6 changes: 5 additions & 1 deletion src/main/java/org/jenkinsci/test/acceptance/po/Job.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,11 @@ public Build scheduleBuild(Map<String, ?> params) {
// so wait for it to be added and then disappear
waitFor(waitFor(By.id("notification-bar"))).until(bar -> !bar.isDisplayed());
} else {
clickLink("Build with Parameters");
if (Boolean.getBoolean("new-job-page.flag.defaultValue")) {
clickButton("Build with Parameters");
Copy link
Member

@jtnord jtnord Dec 19, 2025

Choose a reason for hiding this comment

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

this no longer uses a new page but a dialog - as such the result of this is asynchronous and as such this must be using something like runThenHandleInputDialog(java.lang.Runnable,java.lang.String)

I say something like as I do not beleive there is currently something for "do something that will cause a dialog, when the dialog appears to this, then click the default button").

Copy link
Member

@jtnord jtnord Dec 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

this also needs to wait until it is scheduled = see the code comment above about the notification bar

} else {
clickLink("Build with Parameters");
}
try {
BuildWithParameters paramsPage = new BuildWithParameters(this, new URL(driver.getCurrentUrl()));
paramsPage.enter(parameters, params).start();
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/org/jenkinsci/test/acceptance/po/Parameter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ public By path(String rel) {
return super.path(rel);
}

if (Boolean.getBoolean("new-job-page.flag.defaultValue")) {
return by.xpath(
"//input[@name='name' and @value='%s']/parent::div[@name='parameter']//*[@name='%s']", name, rel);
Comment on lines +57 to +58
Copy link
Member

@jtnord jtnord Dec 19, 2025

Choose a reason for hiding this comment

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

time to add data-testid=parameter-${name} so things can be found.

However the location structure is identical between the existing and experimental page?dialog vs standard page.
The only difference is that we are inside a dialog. In other words this should be the same code for both cases given the search is in the whole document //...

}

String np = driver.findElement(by.xpath("//input[@name='name' and @value='%s']", name))
.getAttribute("path");
String path = np.replaceAll("/name$", "") + "/" + rel;
Expand Down