Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Re enable config overrides test #427

Merged
merged 1 commit into from
Sep 17, 2020
Merged

Re enable config overrides test #427

merged 1 commit into from
Sep 17, 2020

Conversation

ktkrg
Copy link
Contributor

@ktkrg ktkrg commented Sep 17, 2020

Fixes #:
#422

Description of changes:
This change makes the two methods: updateAllRcaConfs and updateRcaConf non static even though they don't mutate any state so that the unit tests for ConfigOverridesApplier can be enabled again.

While it is easy to refactor it out into a static helper, mocking static objects is currently not a simple fix in our codebase as PowerMockRunner is not ignoring log4j classes and is causing initialization issues for a lot of classes during unit testing(#426)

This is a simple fix for enabling the tests until #426 is resolved.

Tests:
./gradlew build

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ktkrg ktkrg self-assigned this Sep 17, 2020
@yojs
Copy link
Contributor

yojs commented Sep 17, 2020

I like how clearly you state why this is done the way it is done. Thanks for the follow up issue creation.

@codecov
Copy link

codecov bot commented Sep 17, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@9a2624c). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #427   +/-   ##
=========================================
  Coverage          ?   71.86%           
  Complexity        ?     2566           
=========================================
  Files             ?      322           
  Lines             ?    14571           
  Branches          ?     1227           
=========================================
  Hits              ?    10472           
  Misses            ?     3707           
  Partials          ?      392           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a2624c...107f3b1. Read the comment docs.

@ktkrg ktkrg merged commit 54e61c5 into master Sep 17, 2020
@ktkrg ktkrg deleted the confOverrideTest branch September 17, 2020 18:33
khushbr added a commit that referenced this pull request Sep 30, 2020
commit 240e046
Author: Joydeep Sinha <[email protected]>
Date:   Thu Sep 24 10:53:01 2020 -0700

    Update PULL_REQUEST_TEMPLATE.md

commit 5bd53d5
Author: Sruti Parthiban <[email protected]>
Date:   Wed Sep 23 11:07:02 2020 -0700

    Persist published actions in sqlite (#433)

    * Persist actions in sqlite

    * Address PR comments

commit dfa9c22
Author: Ruizhen Guo <[email protected]>
Date:   Tue Sep 22 13:05:11 2020 -0700

    Fix cache action to handle unbounded fielddata cache (#432)

commit f67e668
Author: Ruizhen Guo <[email protected]>
Date:   Mon Sep 21 14:48:04 2020 -0700

    enable JVM decider in graph (#431)

commit abc7b12
Author: Vigya Sharma <[email protected]>
Date:   Fri Sep 18 10:47:44 2020 -0700

    Rename config folder to configs (#429)

commit 4eadc4e
Author: Ruizhen Guo <[email protected]>
Date:   Fri Sep 18 10:42:53 2020 -0700

    Add cache clear action in JVM decider (#428)

commit 54e61c5
Author: Karthik Kumarguru <[email protected]>
Date:   Thu Sep 17 11:33:37 2020 -0700

    Re enable config overrides test (#427)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants