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

Don't swallow deprecations and warnings when there is no test context #1320

Merged

Conversation

kategengler
Copy link
Member

@kategengler kategengler commented Jan 9, 2023

Allow them to proceed to the next handler.

Observed this behavior in an application: Deprecations thrown outside of a running test are swallowed by this handler. For example, when running tests on ember-resolver, no @ember/string deprecations are thrown, even though deprecate is called.

I am not sure if this was intentional, but it makes it hard to test that a deprecation isn't thrown by a package.

How can this be tested in the test suite here?

I attempted various ways to test this in the test suite but was unsuccessful. I attempted to add my own registerDeprecationHandler to confirm it was called by the one in setup-context, but I was not able to register my handler in advance of the handler in setup-context, which means mine was always called before the one in setup-context. My attempted test never failed. Is there a good way to inspect the console output? I am mostly concerned with the default logging handler.

This change can be confirmed in a project (as I have in ember-resolver) and also there are a few tests in the suite that trigger deprecations without a context. Before this change, they do not log to the console, after, they do.

@kategengler kategengler added the bug label Jan 9, 2023
Copy link
Contributor

@MelSumner MelSumner left a comment

Choose a reason for hiding this comment

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

I don't know (immediately) how to improve what you have here, but I do think that "Before this change, they do not log to the console, after, they do." is enough for a start.

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.

None yet

3 participants