-
Notifications
You must be signed in to change notification settings - Fork 247
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.
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
base: master
Are you sure you want to change the base?
Fix experimental UI Build With Parameters ATHs #2561
Conversation
|
|
||
| public void start() { | ||
| clickButton("Build"); | ||
| if (Boolean.getBoolean("new-job-page.flag.defaultValue")) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change the experimental pages (and current ones) to use data-testids and do not rely on hard coded paths.
| } else { | ||
| clickLink("Build with Parameters"); | ||
| if (Boolean.getBoolean("new-job-page.flag.defaultValue")) { | ||
| clickButton("Build with Parameters"); |
There was a problem hiding this comment.
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").
There was a problem hiding this comment.
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
| return by.xpath( | ||
| "//input[@name='name' and @value='%s']/parent::div[@name='parameter']//*[@name='%s']", name, rel); |
There was a problem hiding this comment.
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 //...
| public void start() { | ||
| clickButton("Build"); | ||
| if (Boolean.getBoolean("new-job-page.flag.defaultValue")) { | ||
| find(by.xpath("//dialog[@open]//div[@id='bottom-sticker']//button[contains(., 'Build')]")) |
There was a problem hiding this comment.
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.
The new experimental build page changes the


Build With Parameterslink to a button. The ATHs need to be changed in accordance else it will just fail.Before experimental changes
After experimental changes


This PR is to fix ATHs that break due to this change of the
Build With ParamtersTesting done
SImple test that clicks the link now passes
Submitter checklist