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

Improve (and fix some) test finalization #838

Merged
merged 15 commits into from
Jan 9, 2024

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Jan 3, 2024

  • Remove some duplicate checks from pylint
  • Make Mocks private to the module
  • Import module from typing
  • Allow passing the mocker in the constructor
  • Make MockMicrogrid an async context manager
  • Use the MockMicrogrid as an async context manager
  • Improve finalization of power distributing tests
  • Improve finalization of voltage streamer tests
  • Improve finalization of logical meter tests
  • Improve finalization of formula formatter tests
  • Improve finalization of EV charger pool tests
  • Improve finalization of formula composition tests
  • Improve finalization of battery status tests
  • Improve finalization of grid tests
  • Disable pylint check only in one line

@llucax llucax requested a review from a team as a code owner January 3, 2024 12:13
@github-actions github-actions bot added part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) labels Jan 3, 2024
@llucax llucax self-assigned this Jan 3, 2024
@llucax llucax added this to the v1.0.0-rc4 milestone Jan 3, 2024
@llucax llucax force-pushed the mock-grid-cleanup branch 2 times, most recently from 7dc61e9 to afe0ad3 Compare January 4, 2024 10:15
llucax added 3 commits January 4, 2024 16:10
They are already covered by flake8.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Makes the code a bit less verbose, and the types are very well known,
so the origin is obvious.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax llucax force-pushed the mock-grid-cleanup branch from afe0ad3 to bb8253a Compare January 4, 2024 15:12
@@ -4,48 +4,68 @@
# The basic syntax is [label]: [path patterns].
#
# For more details on the configuration please see:
- changed-files:
- any-glob-to-any-file:
Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to be in the wrong commit: 3d9936f

The commit message doesn't seem to mention anything about the labeler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, that was completely unrelated and sneaked in, good catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

so you've just removed the label stuff from here right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup

llucax added 12 commits January 8, 2024 09:56
This is to be able to call `start()` without any arguments, so we can
make the `MockMicrogrid` an async context manager, to make sure it is
properly finalized after it is used.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
It will start when the context is entered and stop when it is left.

This makes the `MockMicrogrid` easier to work with while ensure proper
finalization.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This commit updates many tests that use the `MockMicrogrid` and finalize
it manually to use it as a context manager so finalization is done
automatically.

This commit can be reviewed much more easily hiding differences in
spaces (with `diff -w`, or appending `?w=1` to the GitHub URL).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use a custom async context manager for `_Mocks` so all used components
get automatically finalized.

This commit can be reviewed much more easily hiding differences in
spaces (with `diff -w`, or appending `?w=1` to the GitHub URL).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use the `MockMicrogrid` as a context manager so it gets automatically
finalized. Before no finalization was done in the tests, except for only
one.

This commit can be reviewed much more easily hiding differences in
spaces (with `diff -w`, or appending `?w=1` to the GitHub URL).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use the `MockMicrogrid` as a context manager and an async exit stack so
all used components get automatically finalized. Before no finalization
was done in the tests.

This commit can be reviewed much more easily hiding differences in
spaces (with `diff -w`, or appending `?w=1` to the GitHub URL).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use the `MockMicrogrid` as a context manager and an async exit stack so
all used components get automatically finalized.

This commit can be reviewed much more easily hiding differences in
spaces (with `diff -w`, or appending `?w=1` to the GitHub URL).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use the `MockMicrogrid` as a context manager, a custom async context
manager and an async exit stack so all used components get automatically
finalized.

This commit can be reviewed much more easily hiding differences in
spaces (with `diff -w`, or appending `?w=1` to the GitHub URL).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use the `MockMicrogrid` as a context manager and an async exit stack so
all used components get automatically finalized. Before some
finalization was missing in the tests.

This commit can be reviewed much more easily hiding differences in
spaces (with `diff -w`, or appending `?w=1` to the GitHub URL).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use the `MockMicrogrid` as a context manager and a custom async context
manager for the `BatteryStatusTracker` so all used components get
automatically finalized.

Also uses `time_machine` as a `pytest` fixture when possible to simplify
the tests and avoid having too indent too deep, as the previous approach
used a context manager too, but not an async one, so it can be combined
in the same `with` statement.

This commit can be reviewed much more easily hiding differences in
spaces (with `diff -w`, or appending `?w=1` to the GitHub URL).

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Use an async exit stack to properly finalize the `Grid` object
automatically. The finalization was missing in some tests.

Also uses `time_machine` as a `pytest` fixture when possible to simplify
the tests and avoid having too indent too deep, as the previous approach
used a context manager too, but not an async one, so it can be combined
in the same `with` statement.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
When the disable comment is in its own line, it will disable it for the
rest of the function.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax llucax force-pushed the mock-grid-cleanup branch from bb8253a to 17c17d5 Compare January 8, 2024 08:59
@llucax
Copy link
Contributor Author

llucax commented Jan 8, 2024

Updated!

@llucax llucax requested a review from shsms January 8, 2024 08:59
@llucax llucax added this pull request to the merge queue Jan 9, 2024
Merged via the queue into frequenz-floss:v1.x.x with commit 3e9c892 Jan 9, 2024
14 checks passed
@llucax llucax deleted the mock-grid-cleanup branch January 9, 2024 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.)
Projects
Development

Successfully merging this pull request may close these issues.

2 participants