Skip to content

RUMM-2900 make tests more accurate with url case sensitivity#1263

Merged
xgouchet merged 1 commit into
developfrom
xgouchet/RUMM-2900/url_case_okhttp
Feb 6, 2023
Merged

RUMM-2900 make tests more accurate with url case sensitivity#1263
xgouchet merged 1 commit into
developfrom
xgouchet/RUMM-2900/url_case_okhttp

Conversation

@xgouchet

@xgouchet xgouchet commented Feb 2, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

We used to require all urls to be fully lowercase in our tests assertions, as case sensitivity used to make the tests fail.
After investigation, we noticed that OKHttp automatically standardize the host name to be all lowercasen because the host name by definition (RFC-4343).

According to the original DNS design decision, comparisons on name lookup for DNS queries should be case insensitive.

This PR updates our test to follow closely this principle and OkHttp behavior, by forging random URLs with a lowercase hostname, and a path that is still case sensitive.

@xgouchet
xgouchet requested a review from a team as a code owner February 2, 2023 17:49
@xgouchet
xgouchet requested a review from plousada February 2, 2023 17:49
@codecov-commenter

codecov-commenter commented Feb 2, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1263 (30dfb28) into develop (31b0121) will decrease coverage by 0.10%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1263      +/-   ##
===========================================
- Coverage    82.01%   81.91%   -0.10%     
===========================================
  Files          361      361              
  Lines        12832    12832              
  Branches      2149     2149              
===========================================
- Hits         10524    10511      -13     
- Misses        1655     1660       +5     
- Partials       653      661       +8     
Impacted Files Coverage Δ
...n/com/datadog/android/v2/core/SdkInternalLogger.kt 88.89% <0.00%> (-8.33%) ⬇️
...ndroid/telemetry/internal/TelemetryEventHandler.kt 70.31% <0.00%> (-2.34%) ⬇️
...droid/rum/tracking/ActivityViewTrackingStrategy.kt 86.54% <0.00%> (-1.92%) ⬇️
...al/persistence/file/batch/BatchFileOrchestrator.kt 93.70% <0.00%> (-1.57%) ⬇️
...rsistence/file/batch/PlainBatchFileReaderWriter.kt 81.54% <0.00%> (-1.54%) ⬇️
...lin/com/datadog/android/rum/internal/RumFeature.kt 92.09% <0.00%> (-1.44%) ⬇️
...ain/java/com/datadog/opentracing/PendingTrace.java 59.48% <0.00%> (-0.86%) ⬇️
.../android/rum/internal/monitor/DatadogRumMonitor.kt 90.81% <0.00%> (-0.54%) ⬇️
.../kotlin/com/datadog/android/v2/core/DatadogCore.kt 85.59% <0.00%> (-0.42%) ⬇️
.../android/rum/internal/domain/scope/RumViewScope.kt 95.45% <0.00%> (ø)
... and 3 more

@xgouchet
xgouchet merged commit eae333c into develop Feb 6, 2023
@xgouchet
xgouchet deleted the xgouchet/RUMM-2900/url_case_okhttp branch February 6, 2023 12:12
@xgouchet xgouchet added this to the 1.18.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants