serenity-bdd / serenity-core

Serenity BDD is a test automation library designed to make writing automated acceptance tests easier, and more fun.
http://serenity-bdd.info
Other
722 stars 518 forks source link

PageObject#open() with parameters adds base url twice to page DefaultUrl #2753

Open more7dev opened 2 years ago

more7dev commented 2 years ago

Hello,

I have base url defined in configuration:

webdriver.base.url = http://localhost/base/url

PageObject has DefaultUrl defined:

import net.serenitybdd.core.pages.PageObject; 

@DefaultUrl("/index.php?param={1}") 
public class ExamplePage extends PageObject {

When I want to open page with parameter value:

page.open(withParameters("39"));

Base url is added twice to PageObject url, and webdriver tries to open page:

http://localhost/base/url/base/url/index.php?param=39

Of course, there is no such page.

I have figured out, that net.serenitybdd.core.pages.PageUrls#addBaseUrlTo is called twice on DefaultUrl in the process.

Is that a bug or am I doing something wrong?

wakaleo commented 2 years ago

It looks like a bug. Could you propose a PR to fix it?

more7dev commented 2 years ago

Unfortunately, I'm quite new to Serenity and don`t have any knowledge about its internals.

But I think the problem is in net.serenitybdd.core.pages.PageUrls#getStartingUrl(java.lang.String...).

DefaultUrl is added in the first line, and then later in the second:

public String getStartingUrl(String... parameterValues) {
        String startingUrlTemplate = this.getStartingUrl();
        return this.urlWithParametersSubstituted(startingUrlTemplate, parameterValues);
    }

I suspect that this.urlWithParametersSubstituted should only substitute parameters and not add DefaultUrl, which was added previously. So, the first line should be removed:

private String urlWithParametersSubstituted(String template, String[] parameterValues) {
        String url = this.addBaseUrlTo(template);

This method is also used in PageUrls#getNamedUrl(), but quick look at the code suggests me, that here the problem would be the same and removing this line would also fix it. Am I right?