Skip to content

Commit

Permalink
XWIKI-16276: Send Page by Mail fails when using an email address with…
Browse files Browse the repository at this point in the history 
… < and > characters

* Fix functional test so that it passes when XWiki is running inside a docker container
  • Loading branch information
@vmassol
vmassol committed Apr 17, 2019
1 parent 63dd503 commit 6578f5f
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions ...i-platform-sharepage-test-docker/src/test/it/org/xwiki/sharepage/test/ui/SharePageIT.java
View file
Expand Up @@ -30,7 +30,9 @@
import org.xwiki.sharepage.test.po.ShareDialog;
import org.xwiki.sharepage.test.po.ShareResultDialog;
import org.xwiki.sharepage.test.po.ShareableViewPage;
import org.xwiki.test.docker.junit5.TestConfiguration;
import org.xwiki.test.docker.junit5.UITest;
import org.xwiki.test.docker.junit5.servletengine.ServletEngine;
import org.xwiki.test.ui.TestUtils;

import com.icegreen.greenmail.util.GreenMail;
Expand All @@ -44,7 +46,7 @@
* @version $Id$
* @since 7.0RC1
*/
@UITest(
@UITest(servletEngine = ServletEngine.TOMCAT,

This comment has been minimized.

Sign in to view

Copy link
@tmortagne

tmortagne Dec 2, 2019

Member

I guess this is a mistake ?

This comment has been minimized.

Sign in to view

Copy link
@vmassol

vmassol Dec 2, 2019

Author Member

yes :(

This comment has been minimized.

Sign in to view

Copy link
@vmassol

vmassol Dec 2, 2019

Author Member

We might need some automatic checker to prevent these kind of errors (would be bad to prevent configuring through the java class IMO)...

This comment has been minimized.

Sign in to view

Copy link
@surli

surli Dec 3, 2019

Member

We might need some automatic checker to prevent these kind of errors (would be bad to prevent configuring through the java class IMO)...

I guess we could add this as a Spoon rule to check: servletEngine value in UITest being forbidden except in the rare case where forbiddenEngines value is set. This rule is for cases such as in OfficeImporterIT: https://github.com/xwiki/xwiki-platform/blob/master/xwiki-platform-core/xwiki-platform-office/xwiki-platform-office-test/xwiki-platform-office-test-docker/src/test/it/org/xwiki/officeimporter/test/ui/OfficeImporterIT.java#L57-L63

This comment has been minimized.

Sign in to view

Copy link
@surli

surli Dec 3, 2019

Member

I just created https://jira.xwiki.org/browse/XCOMMONS-1807 for it

This comment has been minimized.

Sign in to view

Copy link
@vmassol

vmassol Dec 3, 2019

Author Member

Thanks @surli I should be able to find some to work on it (unless you prefer to do it).

This comment has been minimized.

Sign in to view

Copy link
@surli

surli Dec 3, 2019

Member

yep I think I'll take it, long time I didn't play with Spoon :)

This comment has been minimized.

Sign in to view

Copy link
@vmassol

vmassol Dec 3, 2019 •
edited

Author Member

Note that I'm still unsure this is the right way. It would be much better to not have to write a verifier for this, ie if it were by design not possible to make mistakes. I'm still trying to think about a different solution (haven't found so far a good one though).

This means finding an easy way to start func tests in our IDEs without having to modify the sources. Passing system properties is quite time consuming and painful IMO so something simpler would be better I think.

This comment has been minimized.

Sign in to view

Copy link
@surli

surli Dec 3, 2019

Member

This means finding an easy way to start func tests in our IDEs without having to modify the sources. Passing system properties is quite time consuming and painful IMO so something simpler would be better I think.

Depends who you're talking too I guess: I personally never change those value, I'm always using the system props.

This comment has been minimized.

Sign in to view

Copy link
@vmassol

vmassol Dec 3, 2019 •
edited

Author Member

Depends who you're talking too I guess: I personally never change those value, I'm always using the system props.

Would be great if you could show me the steps you take in IntelliJ IDEA for that. Because if I can be convinced then it would be enough to disallow configuring through the @UITest annotation.

This comment has been minimized.

Sign in to view

Copy link
@surli

surli Dec 3, 2019

Member

Would be great if you could show me the steps you take in IntelliJ IDEA for that.

So on my side I very rarely execute those tests in IntelliJ, most of the time I execute them in a separated console. Now in IntelliJ I had to play quite often with the test parameters config, so basically my process was:

  1. Trigger test and stop it immediately (to create the config)
  2. Go to the list of test to execute "edit configurations"
  3. Select test and add/edit the properties in VM options

And if it's an option I'm always using, I just edit the template.

This comment has been minimized.

Sign in to view

Copy link
@vmassol

vmassol Dec 3, 2019 •
edited

Author Member

So on my side I very rarely execute those tests in IntelliJ,

Why (I'm curious)? it's much simpler IMO (and faster) and you can debug too.

Go to the list of test to execute "edit configurations"
Select test and add/edit the properties in VM options

Ok I know how that works but that takes like 10x the time it takes me: you need to click several times, you need to remember the system property values (you don't have autocompletion) and you need to redo this for each test (and everytime you recreate your intellij workspace).

And if it's an option I'm always using, I just edit the template.

That seems more interesting. It still takes a lot more time though and you can't quickly switch config.

This comment has been minimized.

Sign in to view

Copy link
@surli

surli Dec 3, 2019

Member

Why (I'm curious)? it's much simpler IMO (and faster) and you can debug too.

Several reasons: one of them is I have the habit to trigger the test and do something else until I get the result, so seems easier to do it in a separate console, so my IDE is not consuming a lot of RAM. Another one I guess is that I had several issues in the past to make them run properly in the IDE. Right now, it's simply because I cannot run anymore any docker tests on my laptop, so I just cannot :)
sshPorts = {
// Open the GreenMail port so that the XWiki instance inside a Docker container can use the SMTP server provided
// by GreenMail running on the host.
Expand Down Expand Up @@ -105,19 +107,26 @@ public void setup(TestUtils setup, TestInfo info)
*/
@Test
@Order(1)
public void shareByEmailWhenNoFromAddress(TestUtils setup) throws Exception
public void shareByEmailWhenNoFromAddress(TestUtils setup, TestConfiguration configuration) throws Exception
{
setup.updateObject("Mail", "MailConfig", "Mail.SendMailConfigClass", 0, "host", "localhost", "port",
"3025", "sendWaitTime", "0");
shareByEmail("=?UTF-8?Q?superadmin?= <noreply@host.testcontainers.internal>", setup);
setup.updateObject("Mail", "MailConfig", "Mail.SendMailConfigClass", 0,
"host", configuration.getServletEngine().getHostIP(),
"port", "3025",
"sendWaitTime", "0",
"from", "");
shareByEmail(String.format("=?UTF-8?Q?superadmin?= <noreply@%s>",
configuration.getServletEngine().getInternalIP()), setup);
}

@Test
@Order(2)
public void shareByEmailWhenFromAddressSpecified(TestUtils setup) throws Exception
public void shareByEmailWhenFromAddressSpecified(TestUtils setup, TestConfiguration configuration) throws Exception
{
setup.updateObject("Mail", "MailConfig", "Mail.SendMailConfigClass", 0, "host", "localhost", "port",
"3025", "sendWaitTime", "0", "from", "noreply@localhost");
setup.updateObject("Mail", "MailConfig", "Mail.SendMailConfigClass", 0,
"host", configuration.getServletEngine().getHostIP(),
"port", "3025",
"sendWaitTime", "0",
"from", "noreply@localhost");
shareByEmail("noreply@localhost", setup);
}

Expand Down

0 comments on commit 6578f5f

Please sign in to comment.