Skip to content

test: More thoroughly test Hash#7078

Merged
HarelM merged 9 commits intomaplibre:mainfrom
CommanderStorm:more-hash-tests
Feb 9, 2026
Merged

test: More thoroughly test Hash#7078
HarelM merged 9 commits intomaplibre:mainfrom
CommanderStorm:more-hash-tests

Conversation

@CommanderStorm
Copy link
Copy Markdown
Member

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!

I think the actual changes in #7073 are getting hard to review.
Lets pull the testcase changes out so that the actual changes are more obvious.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.61%. Comparing base (222db5c) to head (61f3ddb).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7078      +/-   ##
==========================================
+ Coverage   92.58%   92.61%   +0.02%     
==========================================
  Files         289      289              
  Lines       24009    24007       -2     
  Branches     5088     5085       -3     
==========================================
+ Hits        22228    22233       +5     
+ Misses       1781     1774       -7     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@CommanderStorm CommanderStorm changed the title test: more thorughly test Hash test: more thoroughly test Hash Feb 8, 2026
@CommanderStorm CommanderStorm changed the title test: more thoroughly test Hash test: More thoroughly test Hash Feb 8, 2026
Comment thread src/ui/hash.test.ts
test('invalidate hash, only one value', () => {
window.location.hash = '#24';

expect(hash._isValidHash(hash._getCurrentHash())).toBeFalsy();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this was not changed as part of this PR, but if there was a better way to test these internal methods without calling them explicitly, it would've been better...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't know how to do this.

Given that we really want these to be correct, I think not using the public API is better here.

Also, the methods are not marked as private, so technically they are public api ^^

Comment thread src/ui/hash.test.ts
Comment thread src/ui/hash.test.ts
Comment thread src/ui/hash.test.ts Outdated
Comment thread src/ui/hash.test.ts
Comment thread src/ui/hash.test.ts Outdated
Comment thread src/ui/hash.test.ts
Comment thread src/ui/hash.test.ts Outdated
Comment thread src/ui/hash.test.ts
@HarelM
Copy link
Copy Markdown
Collaborator

HarelM commented Feb 8, 2026

Sorry for spamming... I hope this is not too much...

@HarelM HarelM merged commit 0e7fd47 into maplibre:main Feb 9, 2026
26 checks passed
@CommanderStorm CommanderStorm deleted the more-hash-tests branch February 9, 2026 15:19
bigmistqke pushed a commit to bigmistqke/maplibre-gl-js that referenced this pull request Mar 20, 2026
* qee more tests

* more tests

* remove useless test

* simplify a few more things

* fix fmt

* Update src/ui/hash.test.ts

* Update src/ui/hash.test.ts

* Apply suggestion from @CommanderStorm
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.

2 participants