Skip to content

Conversation

@Jerry-Jinfeng-Guo
Copy link
Member

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo commented Oct 1, 2025

Unit test for all functions within the observability module, related to #1136

  • radial check functions
  • meshed check functions

The unit tests are added with still dependency on mathematical topology and YBusStructure. This choice was due to the fact that when trying to wrap certain functionalities from the observability module, any mocking attempt will have to be redone. Therefore, the most straight-forward path is chosen to extend this unit test for the objective of function-level unit testing this module.

Signed-off-by: Jerry Guo <[email protected]>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo self-assigned this Oct 1, 2025
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added feature New feature or request improvement Improvement on internal implementation labels Oct 1, 2025
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo changed the base branch from main to feature/full-observability-check-meshed-network-without-voltage-phasor October 1, 2025 09:37
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added the do-not-merge This should not be merged label Oct 3, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds comprehensive unit tests for individual functions within the observability module, focusing on more granular function-level testing compared to the existing integration tests. The tests cover radial check functions and provide groundwork for future meshed check function testing.

Key changes:

  • Adds unit tests for all major functions in the observability module including scan_network_sensors, prepare_starting_nodes, expand_neighbour_list, assign_independent_sensors_radial, starting_from_node, necessary_condition, and both sufficient condition functions
  • Maintains dependency on mathematical topology and YBusStructure for practical testing
  • Provides extensive test coverage with various scenarios including edge cases, error conditions, and different network topologies

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@mgovers mgovers left a comment

Choose a reason for hiding this comment

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

just a small preliminary review with things i noticed when we went over the code very quickly during one of our recent standups

Signed-off-by: Jerry Guo <[email protected]>
@Jerry-Jinfeng-Guo
Copy link
Member Author

do-not-merge. Only merge after #1136

…eaking in core logic is sound

Signed-off-by: Jerry Guo <[email protected]>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo marked this pull request as ready for review October 7, 2025 14:23
Jerry-Jinfeng-Guo and others added 6 commits October 8, 2025 12:10
Signed-off-by: Jerry Guo <[email protected]>
…-voltage-phasor' into feature/unit-test-observability-check
Signed-off-by: Jerry Guo <[email protected]>
…-voltage-phasor' into feature/unit-test-observability-check
…-voltage-phasor' into feature/unit-test-observability-check
…-voltage-phasor' into feature/unit-test-observability-check
Copy link
Member

@figueroa1395 figueroa1395 left a comment

Choose a reason for hiding this comment

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

I'm currently at Test Observability - complete_bidirectional_neighbourhood_info. I'll continue as soon as I have time. Looks very good so far.

Signed-off-by: Jerry Guo <[email protected]>
Jerry-Jinfeng-Guo and others added 2 commits October 9, 2025 20:15
…-voltage-phasor' into feature/unit-test-observability-check
Signed-off-by: Jerry Guo <[email protected]>
Copy link
Member

@figueroa1395 figueroa1395 left a comment

Choose a reason for hiding this comment

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

Only the last bit missing

Copy link
Member

@figueroa1395 figueroa1395 left a comment

Choose a reason for hiding this comment

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

Done!

Jerry-Jinfeng-Guo and others added 5 commits October 10, 2025 15:48
Signed-off-by: Jerry Guo <[email protected]>
…-voltage-phasor' into feature/unit-test-observability-check
figueroa1395
figueroa1395 previously approved these changes Oct 16, 2025
Copy link
Member

@figueroa1395 figueroa1395 left a comment

Choose a reason for hiding this comment

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

No further comments from me. Ready to be merged when the parent is.

Jerry-Jinfeng-Guo and others added 2 commits October 21, 2025 15:59
…-voltage-phasor' into feature/unit-test-observability-check
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo removed the do-not-merge This should not be merged label Oct 21, 2025
@Jerry-Jinfeng-Guo
Copy link
Member Author

do-not-merge removed and pending merge to main once #1136 is merged.

@sonarqubecloud
Copy link

Base automatically changed from feature/full-observability-check-meshed-network-without-voltage-phasor to main October 21, 2025 15:35
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo dismissed figueroa1395’s stale review October 21, 2025 15:35

The base branch was changed.

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added this pull request to the merge queue Oct 22, 2025
Merged via the queue into main with commit 5b2dbff Oct 22, 2025
31 of 32 checks passed
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo deleted the feature/unit-test-observability-check branch October 22, 2025 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request improvement Improvement on internal implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants