-
Notifications
You must be signed in to change notification settings - Fork 200
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
MockMvc-based tests fail with App Insights enabled #1281
Comments
Hi @littleaj .. I'm back at the office this week and forked the repo to make this change and test it. When I try
I see the build passing badge on the README but it doesn't link to any details to compare my results and the CI. I haven't made any changes yet but want to start in a good state before I do. Am I missing something here? |
@andrejpk, that's interesting. The test reports are in |
Hi @littleaj .. Java 11->8 helped, also had some state that needed a |
Hi @littleaj , I think the PR on this issue is ready. Please let me know if I need to do anything on the CHANGELOG related to this (I wasn't sure how to mark the version in the PR). |
This the same issue as #895 . I was able to implement the recommended solution to do a test setup that stubbed out the App Insights initialization but this isn't a great solution for my project.
Our approach is to build our APIs so we can enable App Insights through configuration where/as needed (by adding it in via our pom.xml), so we are trying to limit our app code to the Spring logging and instrumentation stack (SLF4J, Micrometer). This is working except in this case I need to add App Insights code to my tests need AI-specific code to work around this issue, breaking our approach.
Is it possible to put a null check before in this line to avoid this issue entirely? That would avoid going through the extra plumbing to avoid this issue in my tests.
I'm happy to make a PR on this if the team agrees with this approach.
The other approach would be a simple way to disable App Insights when running under test, but I don't see a way to do that.
The text was updated successfully, but these errors were encountered: