search

charmed-kubernetes / pytest-operator

Apache License 2.0
6 stars 13 forks source link

Increase static check usability by raising instead of returning None #89

Closed sed-i closed 1 year ago

sed-i commented 1 year ago

Statically checking integration tests before attempting to run them can save some time.

85 added a py.typed marker, but this results in a lot of the following:

tests/integration/test_resource_limits.py:38: error: Item "None" of "Optional[Any]" has no attribute "deploy"  [union-attr]
        await ops_test.model.deploy(
              ^
tests/integration/test_resource_limits.py:45: error: Item "None" of "Optional[Any]" has no attribute "wait_for_idle"  [union-attr]
        await ops_test.model.wait_for_idle(status="active", timeout=deploy_timeout)
              ^
tests/integration/test_resource_limits.py:60: error: Item "None" of "Optional[Any]" has no attribute "applications"  [union-attr]
        await ops_test.model.applications[app_name].set_config(custom_limits)
              ^

The changes proposed in this PR are problematic because they breaks backwards compatibility. That being said, I assume that very few (if any) use cases exist relying on getting a None for the model, etc.

sed-i commented 1 year ago

WDYT, @addyess, overall? Does this stand a chance to land in main?

addyess commented 1 year ago

@sed-i seems likely we could get something like this in

addyess commented 1 year ago

Question -- why stop at only .model and model_name?

Others which could be addressed are:

- model_full_name
- model_config

If we're gonna make a breaking change why not go whole-hog?

addyess commented 1 year ago

@sed-i I think I've pounded on this to a point where I think it's not really benefiting anyone anymore.

This package now uses type checking with mypy for internal consistency type checking during the lint stage I've made a number of the previous Optional[] properties return type instead.

Gimme you're thoughts

addyess commented 1 year ago

Lets consider merging this into 1.x branch so we can version it with 1.0.0.b1 such that it's not picked up by normal installs until we've done some testing with it as a beta

sed-i commented 1 year ago

Hey @addyess, could you please re-trigger the workflow?

addyess commented 1 year ago

@sed-i anything else needed here for a 1.0 beta release?

sed-i commented 1 year ago

@addyess looks good to me :)

addyess commented 1 year ago

@sed-i I released https://pypi.org/project/pytest-operator/1.0.0b1/ from the 1.x branch

You'll have to explicitly request the package because pip won't install pre-release packages by default.

Let's try it out on some projects and see it work before going 1.0.0 final release