Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable URL integration tests to run #4531

Merged
merged 6 commits into from
Mar 16, 2021

Conversation

bdukes
Copy link
Contributor

@bdukes bdukes commented Feb 26, 2021

Summary

The DotNetNuke.Tests.Urls project was in a state where the tests did not run. These integration tests load the site in memory to run various tests. One of the prerequisites to run these tests is to have a DNN database that is setup, and that database's connection string needs to be configured in DNN Platform/Tests/App.config (it does not appear that the siteUrl setting is used for this set of tests, so only the database is required).

The configuration of the components in memory was incomplete, which resulted in the tests failing to run. As well, there were some missing dependencies that also caused the tests to fail. I've adjusted the configuration code to reuse the dependency injection container, which appears to have resolved the majority of the issues.

Please note, not all of the tests pass at the moment, but they do run without throwing exceptions. DotNetNuke.Tests.Urls contains three test classes, ExtensionUrlProviderControllerTests, FriendlyUrlTests, and UrlRewriteTests. The first two have been passing without issue for me, but UrlRewriteTests has about 15% of the tests failing. All of the failing tests that I've seen fail have to do with requests that give a 200 response when a 301 is expected. I don't know if this is a legitimate drift in functionality since these tests have been run, or if there's an issue with the integration where the fake request isn't coming across is the expected way.

This PR does not enable these tests in CI (we would need to setup a database during the CI process to do this), so the failing tests shouldn't cause an immediate issue. Ultimately, we should find a way to be running these tests on a regular basis, and especially during the RC period (though there may be some of these tests we don't want to keep).

@bdukes bdukes added this to the 9.9.1 milestone Feb 26, 2021
@valadas
Copy link
Contributor

valadas commented Feb 26, 2021

Oh, this is awesome, I can't review right now but I am very interested in this and looking at the failing tests and maybe add some more, this would help tremendously to pinpoint some oddities we are having with urls and localization :)

@bdukes is there any special local setup to do or the tests just run in Visual Studio Tests Explorer ?

@bdukes
Copy link
Contributor Author

bdukes commented Feb 27, 2021

@valadas so long as the connection string in App.config is valid, you can run the tests from Visual Studio like any other test.

Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it looks good to me in the browser, but I tried different options in the App.Config to make it actually run because this interests me to help with many url issues fixes.

If ever you feel like showing me @bdukes please slack me up, I must be doing something wrong...

Platform/Tests/DotNetNuke.Tests.Integration/DotNetNuke.Tests.Integration.csproj
Platform/Tests/DotNetNuke.Tests.Utilities/DotNetNuke.Tests.Utilities.csproj
@bdukes
Copy link
Contributor Author

bdukes commented Mar 9, 2021

These same changes also enable the tests in the DotNetNuke.Tests.Integration, you just also need to set the siteUrl key in the tests App.Config file.

@mitchelsellers mitchelsellers merged commit a4b6c03 into dnnsoftware:develop Mar 16, 2021
@mitchelsellers mitchelsellers deleted the url-tests branch March 16, 2021 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants