From 9dcc2517b3755c141f05279ec85845aae25ee7bc Mon Sep 17 00:00:00 2001 From: Chris Joosse Date: Tue, 12 Nov 2019 13:30:41 -0800 Subject: [PATCH 1/4] Issue 38940: need automated test to verify field-level errors/warnings in the field, rather than in the banner --- .../components/domain/DomainFieldRow.java | 17 ++++++++++ .../labkey/test/tests/DomainDesignerTest.java | 33 ++++++++++++++----- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/org/labkey/test/components/domain/DomainFieldRow.java b/src/org/labkey/test/components/domain/DomainFieldRow.java index e201d29fb0..84868f7c52 100644 --- a/src/org/labkey/test/components/domain/DomainFieldRow.java +++ b/src/org/labkey/test/components/domain/DomainFieldRow.java @@ -12,6 +12,7 @@ import org.labkey.test.params.FieldDefinition; import org.labkey.test.util.LabKeyExpectedConditions; import org.openqa.selenium.SearchContext; +import org.openqa.selenium.StaleElementReferenceException; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.ui.ExpectedConditions; @@ -518,11 +519,27 @@ public boolean hasFieldError() { return getComponentElement().getAttribute("class").contains("domain-row-border-error"); } + public DomainFieldRow waitForError() + { + getWrapper().shortWait().ignoring(StaleElementReferenceException.class) + .until(driver -> { + return this.hasFieldError(); + }); + return this; + } public boolean hasFieldWarning() { return getComponentElement().getAttribute("class").contains("domain-row-border-warning"); } + public DomainFieldRow waitForWarning() + { + getWrapper().shortWait().ignoring(StaleElementReferenceException.class) + .until(driver -> { + return this.hasFieldWarning(); + }); + return this; + } // conditional formatting and validation options diff --git a/src/org/labkey/test/tests/DomainDesignerTest.java b/src/org/labkey/test/tests/DomainDesignerTest.java index dc568f98f9..7502d06a15 100644 --- a/src/org/labkey/test/tests/DomainDesignerTest.java +++ b/src/org/labkey/test/tests/DomainDesignerTest.java @@ -305,6 +305,13 @@ public void testBlankNameFieldOnAddedField() throws Exception assertTrue("expect error to contain [Please provide a name for each field.] but was[" + hasNoNameError + "]", hasNoNameError.contains("Please provide a name for each field.")); + String warningfieldMessage = noNameRow.setName("&totally not a sketchy sql injection attack") + .waitForWarning() + .detailsMessage(); + String expectedWarning = "New field. Warning: SQL queries, R scripts, and other code are easiest to write when field names only contain combination of letters, numbers, and underscores, and start with a letter or underscore"; + assertTrue("expect error to contain [" + expectedWarning + "] but was[" + warningfieldMessage + "]", + warningfieldMessage.contains(expectedWarning)); + domainDesignerPage.clickCancelAndDiscardChanges(); } @@ -491,15 +498,23 @@ public void testAddFieldsWithReservedNames() throws Exception DomainFieldRow clientFieldWarning = domainFormPanel.addField("select * from table"); domainDesignerPage.clickFinishExpectingError(); - // TODO: Look for warning on row instead of banner. We're not doing warning banners anymore -// String clientWarning = domainDesignerPage.waitForWarning(); -// String multipleIssuesError = domainDesignerPage.waitForError(); -// String expectedErrMsg = "Multiple fields contain issues that need to be fixed. Review the red highlighted fields below for more information."; -// String expectedWarningMsg = " SQL queries, R scripts, and other code are easiest to write when field names only contain combination of letters, numbers, and underscores, and start with a letter or underscore."; -// assertTrue("expect error message to contain [" + expectedErrMsg + "] but was [" + multipleIssuesError + "]", -// multipleIssuesError.contains(expectedErrMsg)); -// assertTrue("expect warning message to contain [" + expectedWarningMsg + "] but was [" + clientWarning + "]", -// clientWarning.contains(expectedWarningMsg)); + String expectedWarnMsg = "New field. Warning: SQL queries, R scripts, and other code are easiest to write when field names only contain combination of letters, numbers, and underscores, and start with a letter or underscore."; + String blargErrMsg = "New field. Error: The field name 'blarg' is already taken. Please provide a unique name for each field."; + String reservedErrMsg = "New field. Error: 'modified' is a reserved field name in 'fieldsWithReservedNamesSampleSet'."; + String modRowDetailsMsg = modifiedRow.waitForError() + .detailsMessage(); + String blarg1DetailsMsg = blarg1.waitForError().detailsMessage(); + String blarg2DetailsMsg = blarg2.waitForError().detailsMessage(); + String clientFieldWarningMsg = clientFieldWarning.waitForWarning().detailsMessage(); + + assertTrue("expect error message to contain [" + reservedErrMsg + "] but was [" + modRowDetailsMsg + "]", + modRowDetailsMsg.contains(reservedErrMsg)); + assertTrue("expect error message to contain [" + blargErrMsg + "] but was [" + blarg1DetailsMsg + "]", + blarg1DetailsMsg.contains(blargErrMsg)); + assertTrue("expect error message to contain [" + blargErrMsg + "] but was [" + blarg2DetailsMsg + "]", + blarg2DetailsMsg.contains(blargErrMsg)); + assertTrue("expect warning message to contain [" + expectedWarnMsg + "] but was [" + clientFieldWarningMsg + "]", + clientFieldWarningMsg.contains(expectedWarnMsg)); assertTrue("expect field error when using reserved field names", modifiedRow.hasFieldError()); assertTrue("expect error for duplicate field names", blarg1.hasFieldError()); From 4c912c18a228539e91c6ae1cec4da4fcfd351c20 Mon Sep 17 00:00:00 2001 From: Chris Joosse Date: Tue, 12 Nov 2019 14:59:01 -0800 Subject: [PATCH 2/4] Issue 38940: cr feedback --- .../components/domain/DomainFieldRow.java | 11 ++------- .../labkey/test/tests/DomainDesignerTest.java | 23 ++++++++++--------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/org/labkey/test/components/domain/DomainFieldRow.java b/src/org/labkey/test/components/domain/DomainFieldRow.java index 84868f7c52..44e7a273ea 100644 --- a/src/org/labkey/test/components/domain/DomainFieldRow.java +++ b/src/org/labkey/test/components/domain/DomainFieldRow.java @@ -12,7 +12,6 @@ import org.labkey.test.params.FieldDefinition; import org.labkey.test.util.LabKeyExpectedConditions; import org.openqa.selenium.SearchContext; -import org.openqa.selenium.StaleElementReferenceException; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.ui.ExpectedConditions; @@ -521,10 +520,7 @@ public boolean hasFieldError() } public DomainFieldRow waitForError() { - getWrapper().shortWait().ignoring(StaleElementReferenceException.class) - .until(driver -> { - return this.hasFieldError(); - }); + getWrapper().waitFor(()-> this.hasFieldError(), WAIT_FOR_JAVASCRIPT); return this; } @@ -534,10 +530,7 @@ public boolean hasFieldWarning() } public DomainFieldRow waitForWarning() { - getWrapper().shortWait().ignoring(StaleElementReferenceException.class) - .until(driver -> { - return this.hasFieldWarning(); - }); + getWrapper().waitFor(()-> this.hasFieldWarning(), WAIT_FOR_JAVASCRIPT); return this; } diff --git a/src/org/labkey/test/tests/DomainDesignerTest.java b/src/org/labkey/test/tests/DomainDesignerTest.java index 7502d06a15..d9b4a555c5 100644 --- a/src/org/labkey/test/tests/DomainDesignerTest.java +++ b/src/org/labkey/test/tests/DomainDesignerTest.java @@ -43,7 +43,9 @@ import java.util.Map; import java.util.stream.Collectors; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.hasItems; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -305,13 +307,14 @@ public void testBlankNameFieldOnAddedField() throws Exception assertTrue("expect error to contain [Please provide a name for each field.] but was[" + hasNoNameError + "]", hasNoNameError.contains("Please provide a name for each field.")); - String warningfieldMessage = noNameRow.setName("&totally not a sketchy sql injection attack") + String warningFieldMessage = noNameRow.setName("&has weird characters that make scripts hard to write") .waitForWarning() .detailsMessage(); String expectedWarning = "New field. Warning: SQL queries, R scripts, and other code are easiest to write when field names only contain combination of letters, numbers, and underscores, and start with a letter or underscore"; - assertTrue("expect error to contain [" + expectedWarning + "] but was[" + warningfieldMessage + "]", - warningfieldMessage.contains(expectedWarning)); + assertThat("expected error", warningFieldMessage, containsString(expectedWarning)); + assertEquals("save button should be disabled with field errors present", "true", + domainDesignerPage.finishButton().getAttribute("disabled")); domainDesignerPage.clickCancelAndDiscardChanges(); } @@ -507,20 +510,18 @@ public void testAddFieldsWithReservedNames() throws Exception String blarg2DetailsMsg = blarg2.waitForError().detailsMessage(); String clientFieldWarningMsg = clientFieldWarning.waitForWarning().detailsMessage(); - assertTrue("expect error message to contain [" + reservedErrMsg + "] but was [" + modRowDetailsMsg + "]", - modRowDetailsMsg.contains(reservedErrMsg)); - assertTrue("expect error message to contain [" + blargErrMsg + "] but was [" + blarg1DetailsMsg + "]", - blarg1DetailsMsg.contains(blargErrMsg)); - assertTrue("expect error message to contain [" + blargErrMsg + "] but was [" + blarg2DetailsMsg + "]", - blarg2DetailsMsg.contains(blargErrMsg)); - assertTrue("expect warning message to contain [" + expectedWarnMsg + "] but was [" + clientFieldWarningMsg + "]", - clientFieldWarningMsg.contains(expectedWarnMsg)); + assertThat("expected warning", clientFieldWarningMsg, containsString(expectedWarnMsg)); + assertThat("expected error", blarg1DetailsMsg, containsString(blargErrMsg)); + assertThat("expected error", blarg2DetailsMsg, containsString(blargErrMsg)); + assertThat("expected error", modRowDetailsMsg, containsString(reservedErrMsg)); assertTrue("expect field error when using reserved field names", modifiedRow.hasFieldError()); assertTrue("expect error for duplicate field names", blarg1.hasFieldError()); assertTrue("expect error for duplicate field names", blarg2.hasFieldError()); assertTrue("expect warning for field name with spaces or special characters", clientFieldWarning.hasFieldWarning()); + assertFalse("'save' button should not be enabled when field errors are present", + domainDesignerPage.finishButton().isEnabled()); domainDesignerPage.clickCancelAndDiscardChanges(); } From 092c05ca6c43a355ec3d6d6a13d1a9765c3feda1 Mon Sep 17 00:00:00 2001 From: Chris Joosse Date: Tue, 12 Nov 2019 15:01:51 -0800 Subject: [PATCH 3/4] Issue 38940: update test method name --- src/org/labkey/test/tests/DomainDesignerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/org/labkey/test/tests/DomainDesignerTest.java b/src/org/labkey/test/tests/DomainDesignerTest.java index d9b4a555c5..826e333e2b 100644 --- a/src/org/labkey/test/tests/DomainDesignerTest.java +++ b/src/org/labkey/test/tests/DomainDesignerTest.java @@ -480,7 +480,7 @@ public void testConfirmNameFieldFromSamplesetNotShown() throws Exception } @Test - public void testAddFieldsWithReservedNames() throws Exception + public void testFieldNameErrors() throws Exception { String sampleSet = "fieldsWithReservedNamesSampleSet"; From ac3ae86afbe864ae0effb75e439f002a66a77bd9 Mon Sep 17 00:00:00 2001 From: Chris Joosse Date: Wed, 13 Nov 2019 12:39:32 -0800 Subject: [PATCH 4/4] remove asserts that expect 'Save' button to be disabled when errors are present --- src/org/labkey/test/tests/DomainDesignerTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/org/labkey/test/tests/DomainDesignerTest.java b/src/org/labkey/test/tests/DomainDesignerTest.java index 826e333e2b..6d234a09eb 100644 --- a/src/org/labkey/test/tests/DomainDesignerTest.java +++ b/src/org/labkey/test/tests/DomainDesignerTest.java @@ -313,8 +313,6 @@ public void testBlankNameFieldOnAddedField() throws Exception String expectedWarning = "New field. Warning: SQL queries, R scripts, and other code are easiest to write when field names only contain combination of letters, numbers, and underscores, and start with a letter or underscore"; assertThat("expected error", warningFieldMessage, containsString(expectedWarning)); - assertEquals("save button should be disabled with field errors present", "true", - domainDesignerPage.finishButton().getAttribute("disabled")); domainDesignerPage.clickCancelAndDiscardChanges(); } @@ -520,8 +518,6 @@ public void testFieldNameErrors() throws Exception assertTrue("expect error for duplicate field names", blarg2.hasFieldError()); assertTrue("expect warning for field name with spaces or special characters", clientFieldWarning.hasFieldWarning()); - assertFalse("'save' button should not be enabled when field errors are present", - domainDesignerPage.finishButton().isEnabled()); domainDesignerPage.clickCancelAndDiscardChanges(); }