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

Changed forbiddenAPIsTest files and made relevant forbidden fixes #450

Merged
merged 6 commits into from
Mar 30, 2022

Conversation

amitgalitz
Copy link
Member

@amitgalitz amitgalitz commented Mar 17, 2022

Signed-off-by: Amit Galitzky [email protected]

Description

  1. Added a new custom signature file for forbiddenAPIsTest
  2. Replaced printStackTrace calls with more specific logger.error()
  3. Added Locale.ROOT too a lot of string.format() calls.
  4. Added Charset.defaultCharset() to calls to read files in testing
  5. Removed ignoreFailures = true for forbiddenAPIsTest, this is pretty much the most long lasting change as build will now fail if we add new code that with format not using Locale instead of stacking up a list of hundreds of warning over time (this can be changed back so only warnings are shown and not failures if it proves to be too constrictive.)

Issues Resolved

resolves #422

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@amitgalitz amitgalitz requested a review from a team March 17, 2022 21:07
ylwu-amzn
ylwu-amzn previously approved these changes Mar 22, 2022
Copy link
Collaborator

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for improving the code style

Signed-off-by: Amit Galitzky <[email protected]>
Signed-off-by: Amit Galitzky <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Mar 28, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.17%. Comparing base (fa00555) to head (dd4c3e9).
Report is 212 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #450      +/-   ##
============================================
- Coverage     78.18%   78.17%   -0.02%     
  Complexity     4162     4162              
============================================
  Files           296      296              
  Lines         17659    17659              
  Branches       1879     1879              
============================================
- Hits          13807    13805       -2     
- Misses         2958     2960       +2     
  Partials        894      894              
Flag Coverage Δ
plugin 78.17% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 4 files with indirect coverage changes

@amitgalitz amitgalitz requested review from kaituo and ylwu-amzn March 28, 2022 20:57
@amitgalitz amitgalitz merged commit 407e061 into opensearch-project:main Mar 30, 2022
@aksingh-es aksingh-es added the infra Changes to infrastructure, testing, CI/CD, pipelines, etc. label Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Changes to infrastructure, testing, CI/CD, pipelines, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Forbidden method invocation: java.lang.String#format(java.lang.String,java.lang.Object[])
5 participants