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

test(api): disable module concatenation in tree-shaking test #3409

Merged
merged 3 commits into from
Nov 15, 2022

Conversation

legendecas
Copy link
Member

Which problem is this PR solving?

Fixes CI failures in #3208 (comment).

Short description of the changes

When small modules are concatenated, variable names are mangled, which fails the matching in the tree-shaking test.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added

@legendecas legendecas changed the title fix(api): disable module concatenation in tree-shaking test test(api): disable module concatenation in tree-shaking test Nov 14, 2022
@legendecas legendecas marked this pull request as ready for review November 14, 2022 08:41
@legendecas legendecas requested a review from a team November 14, 2022 08:41
@codecov
Copy link

codecov bot commented Nov 14, 2022

Codecov Report

Merging #3409 (7f7d8fa) into main (125f11e) will not change coverage.
The diff coverage is n/a.

❗ Current head 7f7d8fa differs from pull request most recent head ce648a8. Consider uploading reports for the commit ce648a8 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3409   +/-   ##
=======================================
  Coverage   93.26%   93.26%           
=======================================
  Files         247      247           
  Lines        7348     7348           
  Branches     1512     1512           
=======================================
  Hits         6853     6853           
  Misses        495      495           

@dyladan
Copy link
Member

dyladan commented Nov 14, 2022

Maybe it would be safer to validate on a name we know won't be manged? Or on the string API_NAME which is a string constant and can't be mangled?

@legendecas
Copy link
Member Author

Maybe it would be safer to validate on a name we know won't be manged? Or on the string API_NAME which is a string constant and can't be mangled?

Yeah, that's another option. The value of API_NAME is not so unique so we still need an anchor to locate the exact string -- and API_NAME itself can be mangled too. So I find the current approach might be the most straightforward one.

@legendecas legendecas merged commit bd56547 into open-telemetry:main Nov 15, 2022
@legendecas legendecas deleted the api/tree-shaking branch November 15, 2022 16:50
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