fix tck tests #521
fix tck tests #521
Conversation
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
...rk-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/AbstractTapChanger.java
Outdated
Show resolved
Hide resolved
...k-store-iidm-impl/src/main/java/com/powsybl/network/store/iidm/impl/PhaseTapChangerImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
| @@ -14,6 +14,6 @@ | |||
| public class SwitchSetRetainedTest extends AbstractSwitchSetRetainedTest { | |||
| @Override | |||
| public void test() { | |||
| // FIXME | |||
| // FIXME when support multi-thread access | |||
There was a problem hiding this comment.
strange to need this for SwitchSetRetainedTest.test ?
There was a problem hiding this comment.
yes but you should try
There was a problem hiding this comment.
introduced in powsybl/powsybl-core@0888755#diff-4aefddbf69f7854323b3fb14e8b37834542e7568077234e370e2cf8821a92dcbR59
why do we need multithreadaccess ? old code?
There was a problem hiding this comment.
I feel like it's useless in this test, it seems to test another class more than the present one. I'll ask about it
...-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/TwoWindingsTransformerTest.java
Show resolved
Hide resolved
...-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/TwoWindingsTransformerTest.java
Outdated
Show resolved
Hide resolved
...-iidm-impl/src/test/java/com/powsybl/network/store/iidm/impl/TwoWindingsTransformerTest.java
Show resolved
Hide resolved
|
Update PR description |
| Integer position = getTapPosition(); | ||
| setTapPosition(position != null ? position + (getLowTapPosition() - oldValue) : null); |
There was a problem hiding this comment.
Why specifically using an Integer?
Would this not work?
| Integer position = getTapPosition(); | |
| setTapPosition(position != null ? position + (getLowTapPosition() - oldValue) : null); | |
| setTapPosition(getTapPosition() + getLowTapPosition() - oldValue); |
| if (tapPosition < attributes.getLowTapPosition() || tapPosition > getHighTapPosition()) { | ||
| throwIncorrectTapPosition(tapPosition, attributes.getLowTapPosition(), getHighTapPosition()); | ||
| } |
There was a problem hiding this comment.
You can probably move nearly the whole content of this method in a common method in AbstractTapChanger (except the last line)
| @@ -14,6 +14,6 @@ | |||
| public class SwitchSetRetainedTest extends AbstractSwitchSetRetainedTest { | |||
| @Override | |||
| public void test() { | |||
| // FIXME | |||
| // FIXME when support multi-thread access | |||
There was a problem hiding this comment.
I feel like it's useless in this test, it seems to test another class more than the present one. I'll ask about it
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
|
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
no
What kind of change does this PR introduce?
bug fix
What is the current behavior?
when the low tap position is set position is not managed
What is the new behavior (if this is a feature change)?
deplacing the position according to a new low tap position set
Does this PR introduce a breaking change or deprecate an API?