Skip to content
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

Gui improvements 28 06 2024 #3762

Open
wants to merge 52 commits into
base: develop
Choose a base branch
from
Open

Conversation

maryvictol
Copy link
Collaborator

@maryvictol maryvictol commented Nov 6, 2024

This PR provides following updates:

@maryvictol maryvictol added the sys/e2e Issues related to the e2e tests automation label Nov 6, 2024
Comment on lines 325 to 327
if (!clusterMenu().context().$$(node()).stream()
.filter(not(master()))
.collect(toList()).isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we simplify this to clusterMenu().context().$$(node()).stream().anyMatch(not(master()))?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -300,4 +320,19 @@ private static Stream<String> prepareExpectedLogMessages(final String pipelineNa
private static LogAO onRunPage() {
return new LogAO();
}

private void checkNodes() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it will be better to use just one forEach() where we check the condition for each node() instead of the same two stream

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -55,7 +55,8 @@ protected StorageRulesTabAO open() {

public StorageRulesTabAO deleteStorageRule(String fileMaskString) {
$(byText(fileMaskString)).closest("tr").findAll(tagName("td")).get(3).find(tagName("a")).shouldHave(text("Delete")).click();
$(className("ant-confirm-title")).shouldHave(text("Do you want to delete rule \"" + fileMaskString + "\"?"));
$$(className("ant-confirm-title")).filter(text("Do you want to delete rule")).first()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add line breaks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment on lines 234 to 248
public ToolSettings clickCustomParameter() {
return clickAddParameter("String parameter");
}

private ToolSettings clickAddParameter(String parameterType) {
$(byId("add-parameter-dropdown-button")).shouldBe(visible).hover();
$(byText(parameterType)).shouldBe(visible).click();
return this;
}

public ToolSettings setName(String name, int parameterIndex) {
SelenideElement nameField = $(byId(String.format("parameters.params.param_%d.name", parameterIndex)));
nameField.click();
setValue(nameField, name);
return this;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we reuse parameter logic from RunParameterAO?

@@ -15,6 +15,7 @@
*/
package com.epam.pipeline.autotests.mixins;

import static com.codeborne.selenide.Condition.exist;
Copy link
Contributor

Choose a reason for hiding this comment

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

Clean up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -127,6 +127,7 @@ public class C {
DEFAULT_CLUSTER_AWS_EBS_TYPE = conf.getProperty("e2e.ui.cluster.aws.ebs.type");
TEST_RUN_TAG = conf.getProperty("e2e.ui.test.run.tag");
DEFAULT_CLOUD_REGION = conf.getProperty("e2e.ui.default.cloud.region");
SSH_CLOUD_REGION = conf.getProperty("e2e.ssh.cloud.region");
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we should add this config to the default.conf too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sys/e2e Issues related to the e2e tests automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants