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

NEWRELIC-658 check expected error config for noticeError api #1014

Merged
merged 2 commits into from
Oct 4, 2022

Conversation

tbradellis
Copy link
Contributor

Overview

The changes here force the noticeError API to check the expected error configuration when using APIs that do not explicitly set the expected boolean.
Previously, we just passed false all the way through and the expected boolean only gets set if the caller uses the API that includes the boolean.

Related Github Issue

Include a link to the related GitHub issue, if applicable

Testing

Tested with local apps and config.
This will add new tests.

Checks

[ yes] Are your contributions backwards compatible with relevant frameworks and APIs?
[ no] Does your code contain any breaking changes? Please describe.
[ no] Does your code introduce any new dependencies? Please describe.

@tbradellis tbradellis marked this pull request as draft September 21, 2022 14:44
@tbradellis tbradellis force-pushed the expected-errors-notice-error-api branch 2 times, most recently from a5a4f31 to 25ec781 Compare September 28, 2022 23:15
@tbradellis tbradellis marked this pull request as ready for review September 29, 2022 04:42
@tbradellis tbradellis self-assigned this Sep 29, 2022
@tbradellis tbradellis force-pushed the expected-errors-notice-error-api branch from 315a0d2 to 9741701 Compare September 29, 2022 23:20
@tbradellis tbradellis force-pushed the expected-errors-notice-error-api branch from be2f913 to 69f4488 Compare September 30, 2022 22:37
@tbradellis tbradellis force-pushed the expected-errors-notice-error-api branch from ff54fcc to 338e527 Compare October 3, 2022 22:30
use placeholder for config

format

rm double check

still check for null

corrections for config check

tweak to fix Supportability metrics

account for null throwable

rm stand-in test

handle null throwable

refactor no expected strings

remove hack

reformat file. drop unused var

tests for config+expected+noticeErrorAPI

checkout APITest from main

must close environment holder

call for the correct count

optimize imports

remove bad test

remove invalid test

review revisions. clean code

fix supportability check

refactor no expectedByAPI bool
@tbradellis tbradellis force-pushed the expected-errors-notice-error-api branch from e4ec0ec to 3e46800 Compare October 4, 2022 17:38
Copy link
Contributor

@jasonjkeller jasonjkeller left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this. LGTM!

@tbradellis tbradellis changed the title check expected error config for noticeError api NEWRELIC-658 check expected error config for noticeError api Oct 4, 2022
@tbradellis tbradellis merged commit 27e13a7 into main Oct 4, 2022
@tbradellis tbradellis deleted the expected-errors-notice-error-api branch October 4, 2022 20:37
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