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

Fix running tests with all deprecations enabled. #20693

Merged
merged 4 commits into from May 9, 2024

Conversation

kategengler
Copy link
Member

The refactor to put the QUnit parameter directly into the EmberENV instead of using process.env to set the QUnit parameter resulted in the property being a string instead of a boolean

QUnit parameter directly into the EmberENV instead of using process.env
to set the QUnit parameter resulted in the property being a string
instead of a boolean
@kategengler
Copy link
Member Author

@ef4 Can you think of a good way to guard this with a test? It is the kind of thing that can break without noticing, easily.

@ef4
Copy link
Contributor

ef4 commented May 6, 2024

I should have hand-checked this case. Now that you've fixed it, I think it falls into the area of test infrastructure that it's more economical to trust and hand-check-when-you-touch-it than to continuously test.

But if we want to test it, I think it would require making a test that leaves detectable traces in the log output (like console.log("I checked that all deprecations are enabled"), adapting the test runner job to notice that output and make it available to subsequent jobs, and having a final job that looks at the output of all the matrix runs to assert that at least one produced the expected signature.

@kategengler
Copy link
Member Author

Yeah that's the only style I could come up with too -- I agree that we just need to keep an eye on it.

@kategengler kategengler merged commit 47f3eeb into main May 9, 2024
27 checks passed
@kategengler kategengler deleted the kg-fix-run-with-deprecations-enabled branch May 9, 2024 20:54
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

3 participants