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

Avoid compressing constructor names when running tests #200

Conversation

johannesodland
Copy link
Contributor

Adding this PR for discussion.

Microbundle compresses the build with terser by default. This results in CSSOM constructor names being replaced with t or similarly compressed names, which causes some subtests to fail.

Subtests using the css-typed-om test-helper.js fail, as the test-helper uses the constructor name to determine how to compare arguments.

I don't think microbundle allows for configuring terser directly. Configuration seems to be limited to enabling or disabling compression. The only available option I can see is building an uncompressed bundle for testing. Otherwise we would need to configure rollup, babel and terser ourself to ensure terser doesn't compress constructor names.

@bramus
Copy link
Collaborator

bramus commented Jan 29, 2024

Feels a bit weird to run the tests with a different build, as there can be subtle differences that work / don’t work in the regular type of build. Ideally, the build used in tests is also the one that ships to users.

Or won’t this change affect regular users?

@johannesodland
Copy link
Contributor Author

Feels a bit weird to run the tests with a different build, as there can be subtle differences that work / don’t work in the regular type of build. Ideally, the build used in tests is also the one that ships to users.

Or won’t this change affect regular users?

I agree.

Preferably the constructor names shouldn't be compressed, as users should also be able to check the constructor name.

As the build is now some tests are failing due to the compression, and as long as these tests are failing we won't be able to notice any regression on the functionality that is under test, such as view-timeline-get-set-range.html.

For the long term I think we need more control over the build. Maybe we need a different build setup.

Until then it's a choice between testing the same build as we ship while loosing out on some tests, or having better test coverage and the ability to discover regressions.

@johannesodland
Copy link
Contributor Author

Closing as this is addressed in #219, #224 and following PRs

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.

None yet

2 participants