[tests] Fix UrlEscaping_Bug43411 to actually run and assert correctly#11523
Merged
Conversation
Two issues with this test on main: 1. The method was declared `void UrlEscaping_Bug43411 ()` (non-public). NUnitLite silently skipped non-public `[Test]` methods, so the test has been effectively dead for years. Discovered while migrating to stock NUnit 3 via `dotnet test` in PR #11224, where NUnit 3 surfaces the method and it fails. 2. The assertion compared the request URL against `Uri.ToString ()`, which returns the human-readable form and unescapes safe characters like `%20` to a literal space. The canonical, encoded form is `Uri.AbsoluteUri` — switch to that so percent-encoding is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a previously undiscovered AndroidMessageHandler integration test so it is run by NUnit and validates URL escaping using the encoded URI representation.
Changes:
- Makes
UrlEscaping_Bug43411public so the test runner discovers it. - Updates the assertion to compare against
Uri.AbsoluteUriinstead ofUri.ToString(). - Adds a short comment explaining the
ToString()vsAbsoluteUridistinction.
- Make `UrlEscaping_Bug43411` (and the `UrlEscaping_TestUrl` helper) `async Task` and `await` `SendAsync` instead of blocking with `.Wait ()`. - The previous listener callback unconditionally set `failed = true`, so `Assert.IsNull (failed)` always failed (`Expected: null, But was: True`). Capture `HttpListenerContext.Request.RawUrl` on the server side instead and compare it against the expected `Uri.PathAndQuery` — this is the verification the test was clearly meant to do (confirm that percent-encoded characters survive the round trip to the server). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Member
Author
|
New test is passing: 
|
jonathanpeppers
added
the
ready-to-review
simonrozsival
approved these changes
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two issues with
UrlEscaping_Bug43411inAndroidMessageHandlerIntegrationTestsonmain:The test has been silently dead for years. The method was declared
void UrlEscaping_Bug43411 ()(non-public). NUnitLite — the legacy in-process runner used by the device test suite — silently skipped non-public[Test]methods, so this test never ran. Nobody noticed because the failure mode was "silently never runs."Discovered while migrating to stock NUnit 3 via
dotnet testin [WIP] Dogfooddotnet testfor device tests #11224, where NUnit 3 actually surfaces the method and it fails.The assertion was wrong. It compared the request URL against
request.RequestUri.ToString (), but per the .NET docsUri.ToString ()returns the "human-readable" form and unescapes safe characters like%20→ literal space. SoAssert.AreEqual ("http://localhost:8810/?example=value%20_value", request.RequestUri.ToString ())fails because the actual value containsvalue _valuewith a space.The correct property is
Uri.AbsoluteUri, which returns the canonical encoded form and preserves percent-encoding.Changes
UrlEscaping_Bug43411publicso NUnit discovers it. The helperUrlEscaping_TestUrlstays non-public.UrlEscaping_TestUrl, switch fromrequest.RequestUri.ToString ()torequest.RequestUri.AbsoluteUri, with a brief comment explaining why.See #11224 for context on the NUnit 3 migration that surfaced this.