Skip to content

SONARJAVA-4634 Test for duplicates in credential method json file#5132

Merged
romainbrenguier merged 2 commits intomasterfrom
romain/test-credential-methods
May 12, 2025
Merged

SONARJAVA-4634 Test for duplicates in credential method json file#5132
romainbrenguier merged 2 commits intomasterfrom
romain/test-credential-methods

Conversation

@romainbrenguier
Copy link
Copy Markdown
Contributor

@romainbrenguier romainbrenguier commented May 7, 2025

SONARJAVA-4634

Adding a test for duplicated entries found a few cases where we could have false positives because of wildcard usage.

@romainbrenguier romainbrenguier changed the title SONARJAVA-5464 Test for duplicates in credential method json file SONARJAVA-4634 Test for duplicates in credential method json file May 7, 2025
@romainbrenguier romainbrenguier force-pushed the romain/test-credential-methods branch from 629a216 to 9799aad Compare May 7, 2025 14:25
@romainbrenguier romainbrenguier marked this pull request as ready for review May 7, 2025 14:52
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","*","int","java.lang.String","java.lang.String","int"],"indices":[5]},
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","*","int","java.lang.String","java.lang.String","int","java.lang.String"],"indices":[5]},
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","*","java.lang.String"],"indices":[3]},
{"cls":"redis.clients.jedis.JedisSentinelPool","name":"JedisSentinelPool","args":["java.lang.String","java.util.Set","org.apache.commons.pool2.impl.GenericObjectPoolConfig","java.lang.String"],"indices":[3]},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the entry with * that was removed here was covering both line 7441 and line 7443, maybe instead of replacing this entry we could remove line 7443 as it's redundant?
I'm not sure if the intent of these entries is to prevent or to fix, the * approach would cover possible future new signatures, while being more specific with the naming allows to intervene only when needed.
I think we might want an opinion from Hendrik here, as he's the main contributor to this list

@romainbrenguier romainbrenguier force-pushed the romain/test-credential-methods branch from ddb2035 to b8c93f6 Compare May 12, 2025 08:06
@romainbrenguier romainbrenguier force-pushed the romain/test-credential-methods branch from b8c93f6 to 75a361d Compare May 12, 2025 09:12
@romainbrenguier romainbrenguier merged commit bf33ace into master May 12, 2025
16 checks passed
@romainbrenguier romainbrenguier deleted the romain/test-credential-methods branch May 12, 2025 14:10
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