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

Feature/refactor test power sensor #419

Merged
merged 2 commits into from Nov 1, 2023

Conversation

mgovers
Copy link
Member

@mgovers mgovers commented Nov 1, 2023

This ensures that code changes in #409 remain small, reviewable and isolated

I could still refactor even further, but I believe that we can do that at a later stage if needed. At least most of the near-identical code duplications are now removed

Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers added the improvement Improvement on internal implementation label Nov 1, 2023
@mgovers mgovers self-assigned this Nov 1, 2023
Copy link

sonarcloud bot commented Nov 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@TonyXiang8787
Copy link
Member

@mgovers So this is a direct PR to main and ready for review?

@petersalemink95
Copy link
Member

Is it correct that there are more tests removed then added? Seems like you're testing less now

@mgovers
Copy link
Member Author

mgovers commented Nov 1, 2023

@mgovers So this is a direct PR to main and ready for review?

yes indeed

Is it correct that there are more tests removed then added? Seems like you're testing less now

i combined the tests for power sensor on {Shunt, Load} as well as {Generator, Branch_from, Branch_to, Source}, because they were exactly the same (aside from typos in the comments). The way I tested this was by creating a for loop that loops over the terminal types for each of those sets, and then do a regex match on the full test case and i fixed the difference between the comments - and that worked as expected.

@TonyXiang8787 TonyXiang8787 merged commit 33ae883 into main Nov 1, 2023
27 checks passed
@TonyXiang8787 TonyXiang8787 deleted the feature/refactor-test-power-sensor branch November 1, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement on internal implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants