OFFI-78: Support different base path for OC app #386
Conversation
…s." code analyzer violation
| @@ -481,7 +481,7 @@ private async Task SetupAsync() | |||
|
|
|||
| _dockerConfiguration = TestConfigurationManager.GetConfiguration<DockerConfiguration>(); | |||
|
|
|||
| var resultUri = await _currentSetupSnapshotManager.RunOperationAndSnapshotIfNewAsync(async () => | |||
There was a problem hiding this comment.
Please keep this. The setup operation needs to be able to create the URL the test should start from.
There was a problem hiding this comment.
Okay, and in what form is it used currently? How can UI tests be started on different URLs currently? I can't seem to find it.
There was a problem hiding this comment.
It's this feature you removed here. The URL returned by the setup operation will be the starting URL of the test.
There was a problem hiding this comment.
I luled, obviously this is what it was using it, but it is not anymore. So let me rephrase my question. Why is it needed to use this URL to navigate to the relative URL at the end of the setup? Is there a reason for that? If so, I don't think resultUri.PathAndQuery is the right way of doing it.
So e.g. what you say is if you set a URL of https://example.com/path/ to start the test on, you would get the relative Url https://example.com/path/path at line await _context.GoToRelativeUrlAsync(resultUri.PathAndQuery); which is obviously not okay.
So the correct navigation step is to navigate to /, which means resultUri isn't needed.
There was a problem hiding this comment.
Why would it be correct to always navigate to /? The reason we have the setup operation return a URL is precisely because that's not necessarily the case: the URL it returns is the start URL of the test. This is the URL that'll be open when control arrives at the beginning of a test's code. The setup operation is not necessarily just filling out the setup screen, it can be anything to initialize the app to a testable state.
There was a problem hiding this comment.
I swear I did not spend several hours on this.
There was a problem hiding this comment.
Uhm this actually won't work because we also need a different base path during setup.
There was a problem hiding this comment.
Okay nvm, I can still do it with this, but now it just seems like another solution, and not necessarily a better solution. But I can do it this way if you say so.
There was a problem hiding this comment.
Utilizing an existing mechanism, just also earlier, seems like the solution to go, yes.
|
|
||
| /// <summary> | ||
| /// Gets or sets the base path of the application if mapped to a different URL. For example, if the Orchard Core | ||
| /// application is mapped to "/cms" then this should be "/cms/". The / is important at the end, otherwise during |
There was a problem hiding this comment.
You can make sure this happens with TrimEnd('/') + "/'".
Lombiq.Tests.UI/BasicOrchardFeaturesTesting/MediaOperationsTestingUITestContextExtensions.cs
Show resolved
Hide resolved
Lombiq.Tests.UI/BasicOrchardFeaturesTesting/MediaOperationsTestingUITestContextExtensions.cs
Outdated
Show resolved
Hide resolved
Lombiq.Tests.UI/BasicOrchardFeaturesTesting/MediaOperationsTestingUITestContextExtensions.cs
Outdated
Show resolved
Hide resolved
OFFI-78