Allow configuration overrides #26
Conversation
filipesperandio
changed the title
filipesperandio
force-pushed
the
fe/enfore-config-json
branch
from
acaec15 to
3fa5ea7
Compare
filipesperandio
changed the title
codeclimate-hermes
requested review from
toddmazierski
and removed request for
codeclimate-hermes
…ed using config.json
filipesperandio
force-pushed
the
fe/enfore-config-json
branch
from
955bd8b to
410439b
Compare
| @@ -48,7 +48,7 @@ public void limit_path_included_within_analysis() throws Exception { | |||
|
|
|||
| @Test | |||
| public void include_all_files_by_default() throws Exception { | |||
| App.execute(new String[]{}, system); | |||
| App.execute(system); | |||
There was a problem hiding this comment.
Are there any test cases for when an exception is caught/logged in App.execute()?
There was a problem hiding this comment.
Adding that.
| @Test | ||
| public void fetch_tests_patterns() throws Exception { | ||
| Config config = Config.gson().fromJson("{\"config\":{\"tests_patterns\":[\"src/test/**\",\"src/test2/**\"]}}", Config.class); | ||
| assertThat(config.getTestsPatterns()).isEqualTo("{src/test/**,src/test2/**}"); |
There was a problem hiding this comment.
Should we also add a test case for when tests_patterns is null (or missing) in the configuration file?
| @@ -30,6 +29,7 @@ public JsonReport(String baseDir) { | |||
| this.gson = new GsonFactory().create(); | |||
| } | |||
|
|
|||
|
|
|||
| public class ReportFactory extends org.sonarlint.cli.report.ReportFactory { | ||
| public ReportFactory(Charset charset) { | ||
| super(charset); | ||
| } |
There was a problem hiding this comment.
Think there's any worth adding tests for this?
There was a problem hiding this comment.
I am actually fine with the SanityCheckTest testing this from an integration perspective.
Having the test in a higher level allows me to easy refactor and move stuff around without needing to update the tests as often.
| } | ||
| @Override | ||
| public int run() { | ||
| return super.run(); |
There was a problem hiding this comment.
Is it necessary to override this function if we're just delegating to the parent anyways?
There was a problem hiding this comment.
Needed to change the accessor to be public so we don't have to keep adding classes to that org.sornarlint.cli package :(
|
@toddmazierski Adjustments made! |
tests_patternscharset