You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by GitBox <gi...@apache.org> on 2022/05/17 19:36:51 UTC

[GitHub] [directory-studio] roubert commented on a diff in pull request #34: DIRSTUDIO-1298: Add address editor UI test

roubert commented on code in PR #34:
URL: https://github.com/apache/directory-studio/pull/34#discussion_r875190111


##########
tests/test.integration.ui/src/main/java/org/apache/directory/studio/test/integration/ui/EntryEditorTest.java:
##########
@@ -567,6 +568,76 @@ public void testTextValueEditor( TestLdapServer server ) throws Exception
     }
 
 
+    /**
+     * DIRSTUDIO-1298: The RFC 4517 Postal Address value editor en-/decoding is incomplete
+     */
+    @ParameterizedTest
+    @LdapServersSource
+    public void testAddressValueEditor( TestLdapServer server ) throws Exception
+    {
+        connectionsViewBot.createTestConnection( server );
+        browserViewBot.selectEntry( path( BJENSEN_DN ) );

Review Comment:
   Just an idea, not important: Maybe it could have some added value if you instead began the testing with an entry that already has the `postalAddress` field set, like eg. _Aaccf Amar_ from the export test, so that you get to first test that LDAP data gets correctly decoded before going into the added complications of text entry and encoding.



##########
tests/test.integration.ui/src/main/java/org/apache/directory/studio/test/integration/ui/EntryEditorTest.java:
##########
@@ -567,6 +568,76 @@ public void testTextValueEditor( TestLdapServer server ) throws Exception
     }
 
 
+    /**
+     * DIRSTUDIO-1298: The RFC 4517 Postal Address value editor en-/decoding is incomplete
+     */
+    @ParameterizedTest
+    @LdapServersSource
+    public void testAddressValueEditor( TestLdapServer server ) throws Exception
+    {
+        connectionsViewBot.createTestConnection( server );
+        browserViewBot.selectEntry( path( BJENSEN_DN ) );
+
+        EntryEditorBot entryEditorBot = studioBot.getEntryEditorBot( BJENSEN_DN.getName() );
+        entryEditorBot.activate();
+        String dn = entryEditorBot.getDnText();
+        assertEquals( "DN: " + BJENSEN_DN.getName(), dn );
+        assertEquals( 8, entryEditorBot.getAttributeValues().size() );
+        assertEquals( "", modificationLogsViewBot.getModificationLogsText() );
+
+        // add postalAddress attribute and verify value is correctly encoded
+        entryEditorBot.activate();
+        NewAttributeWizardBot wizardBot = entryEditorBot.openNewAttributeWizard();
+        assertTrue( wizardBot.isVisible() );
+        wizardBot.typeAttributeType( "postalAddress" );
+        AddressEditorDialogBot addressEditorDialogBot = wizardBot.clickFinishButtonExpectingAddressEditor();
+        assertTrue( addressEditorDialogBot.isVisible() );
+        addressEditorDialogBot.setText( "1234 Main St.\nAnytown, CA 12345\nUSA" );

Review Comment:
   Is it really correct to use plain `\n` here? I know precious little about Eclipse RCP but the very existence of such things as `BrowserCoreConstants.LINE_SEPARATOR` makes me wary and I'd like to make sure that you're sure that this `setText()` call really is the right way to do it.



##########
tests/test.integration.ui/src/main/java/org/apache/directory/studio/test/integration/ui/EntryEditorTest.java:
##########
@@ -567,6 +568,76 @@ public void testTextValueEditor( TestLdapServer server ) throws Exception
     }
 
 
+    /**
+     * DIRSTUDIO-1298: The RFC 4517 Postal Address value editor en-/decoding is incomplete
+     */
+    @ParameterizedTest
+    @LdapServersSource
+    public void testAddressValueEditor( TestLdapServer server ) throws Exception
+    {
+        connectionsViewBot.createTestConnection( server );
+        browserViewBot.selectEntry( path( BJENSEN_DN ) );
+
+        EntryEditorBot entryEditorBot = studioBot.getEntryEditorBot( BJENSEN_DN.getName() );
+        entryEditorBot.activate();
+        String dn = entryEditorBot.getDnText();
+        assertEquals( "DN: " + BJENSEN_DN.getName(), dn );
+        assertEquals( 8, entryEditorBot.getAttributeValues().size() );
+        assertEquals( "", modificationLogsViewBot.getModificationLogsText() );
+
+        // add postalAddress attribute and verify value is correctly encoded
+        entryEditorBot.activate();
+        NewAttributeWizardBot wizardBot = entryEditorBot.openNewAttributeWizard();
+        assertTrue( wizardBot.isVisible() );
+        wizardBot.typeAttributeType( "postalAddress" );
+        AddressEditorDialogBot addressEditorDialogBot = wizardBot.clickFinishButtonExpectingAddressEditor();
+        assertTrue( addressEditorDialogBot.isVisible() );
+        addressEditorDialogBot.setText( "1234 Main St.\nAnytown, CA 12345\nUSA" );
+        addressEditorDialogBot.clickOkButton();
+        assertEquals( 9, entryEditorBot.getAttributeValues().size() );
+        assertTrue(
+            entryEditorBot.getAttributeValues().contains( "postalAddress: 1234 Main St., Anytown, CA 12345, USA" ) );
+        modificationLogsViewBot.waitForText( "add: postalAddress\npostalAddress: 1234 Main St.$Anytown, CA 12345$USA" );
+
+        // verify value is correctly decoded
+        addressEditorDialogBot = entryEditorBot.editValueExpectingAddressEditor( "postalAddress",
+            "1234 Main St., Anytown, CA 12345, USA" );
+        assertTrue( addressEditorDialogBot.isVisible() );
+        assertEquals( "1234 Main St.\nAnytown, CA 12345\nUSA", addressEditorDialogBot.getText() );
+        addressEditorDialogBot.clickCancelButton();
+
+        // edit value with a complex address and verify value is correctly encoded
+        addressEditorDialogBot = entryEditorBot.editValueExpectingAddressEditor( "postalAddress",
+            "1234 Main St., Anytown, CA 12345, USA" );
+        assertTrue( addressEditorDialogBot.isVisible() );
+        assertEquals( "1234 Main St.\nAnytown, CA 12345\nUSA", addressEditorDialogBot.getText() );
+        // TODO: $1,000,000 Sweepstakes
+        addressEditorDialogBot.setText( "1,000,000 Sweepstakes\nPO Box 1000000\nAnytown, CA 12345\nUSA" );

Review Comment:
   I don't think it adds any additional value to test calling `setText()` twice with just slightly different addresses, that just makes the test code longer, the exact workings of the encoding and decoding is already covered by the unit test, the purpose of the integration test is just to verify that the encoding and decoding functions are called where they should be called.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@directory.apache.org
For additional commands, e-mail: dev-help@directory.apache.org