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

Make options.hosts case insensitive #3653

Merged
merged 1 commit into from
Mar 18, 2024
Merged

Make options.hosts case insensitive #3653

merged 1 commit into from
Mar 18, 2024

Conversation

joanlopez
Copy link
Contributor

What?

It updates the Hosts type constructor NewHosts store source keys (i.e. hosts) lowercased, to consider them case insensitive. The internal trie, and the Hosts.Match is already considering them insensitive, as they're treated as lowercase.

Why?

Because otherwise, options.hosts doesn't work well with non-lowercase hosts.

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

Closes #3651

Also related with #3620

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.50%. Comparing base (9ee0188) to head (15cb813).

❗ Current head 15cb813 differs from pull request most recent head fe7a778. Consider uploading reports for the commit fe7a778 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3653      +/-   ##
==========================================
- Coverage   73.58%   73.50%   -0.09%     
==========================================
  Files         277      275       -2     
  Lines       20244    20242       -2     
==========================================
- Hits        14896    14878      -18     
- Misses       4400     4410      +10     
- Partials      948      954       +6     
Flag Coverage Δ
macos ?
ubuntu 73.50% <100.00%> (-0.01%) ⬇️
windows ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mstoykov mstoykov added this to the v0.50.0 milestone Mar 18, 2024
@joanlopez joanlopez merged commit f27cca5 into grafana:master Mar 18, 2024
24 checks passed
joanlopez added a commit that referenced this pull request Mar 18, 2024
oleiade added a commit that referenced this pull request Mar 25, 2024
* Add k6 v0.50.0 release notes

* Add changes for the goja refactors in browser

* Update release notes/v0.50.0.md

Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>

* Add browser#1163 to release notes

* Add browser#1205 to release notes (#3584)

* Add 1215 to release notes

* Update release notes/v0.50.0.md

Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>

* Add screenshot upload to release notes

* Update wording for screenshot upload update

* Add browser 1209 to release notes (#3604)

* Add browser 1217 to release notes (#3603)

* Add browser 850 and Go 1.20 PRs to release notes (#3605)

* Add browser 1220 and 1221 to release notes

* Add/browser 1112 to release notes (#3624)

* Add grafana/xk6-browser#1112

* Rephrase browser 1112 to release notes

Co-authored-by: Ankur <ankur.agarwal@grafana.com>

* Add better error pr for browser evaluate APIs

* Add browser testRunId inject prs to release note

* Update the verb

* Add details of browser#1238 to release notes

* Add details of browser#1097 to release notes

* Apply suggestions from code review

Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>

* JWK, options.cloud and some other changelogs

* Link webcrypto JWK import/export to PR

* Add browser#1241 to release notes

* Update the browser#1097 release notes

The API has changed so that it requires working with the experimental
fs module to upload files from the local file system.

* Add browser 986 (#3652)

* Add browser 986

* Update add browser 1246

Co-authored-by: Ankur <ankur.agarwal@grafana.com>

* Add k6/timers stabilization release notes

* Add #3653 to release notes

* Apply suggestions from code review

Co-authored-by: Heitor Tashiro Sergent <heitortsergent@gmail.com>

* Add notice for async browser API breaking change

* Apply suggestions from code review

Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>

* Add await explanation on non thenable

* Add release changes top-level summary

* Apply suggestions from code review

Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>

* Remove placeholders

---------

Co-authored-by: ankur22 <ankur.agarwal@grafana.com>
Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>
Co-authored-by: Oleg Bespalov <oleg.bespalov@grafana.com>
Co-authored-by: Mihail Stoykov <M.Stoikov@gmail.com>
Co-authored-by: Joan López de la Franca Beltran <joanjan14@gmail.com>
Co-authored-by: Heitor Tashiro Sergent <heitortsergent@gmail.com>
Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hosts options doesn't work well with non-lowercase hosts
4 participants