From 742eb2465c6f5ce75ab5460b7a7907a206adaa41 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Wed, 6 Sep 2023 14:00:06 +0200 Subject: [PATCH 1/9] updated to declarative test api --- README_INTERFACE_TESTS.md | 125 +++++++++++------- interfaces/ingress/v1/charms.yaml | 1 + .../v1/interface_tests/provider_tests.py | 69 ---------- .../v1/interface_tests/test_provider.py | 74 +++++++++++ .../v0/interface_tests/provider_tests.py | 43 ------ .../v0/interface_tests/test_provider.py | 34 +++++ .../{requirer_tests.py => test_requirer.py} | 34 ++--- pyproject.toml | 16 +-- run_matrix.py | 2 +- 9 files changed, 205 insertions(+), 193 deletions(-) delete mode 100644 interfaces/ingress/v1/interface_tests/provider_tests.py create mode 100644 interfaces/ingress/v1/interface_tests/test_provider.py delete mode 100644 interfaces/openfga/v0/interface_tests/provider_tests.py create mode 100644 interfaces/openfga/v0/interface_tests/test_provider.py rename interfaces/openfga/v0/interface_tests/{requirer_tests.py => test_requirer.py} (61%) diff --git a/README_INTERFACE_TESTS.md b/README_INTERFACE_TESTS.md index 607bc284..a46b47cd 100644 --- a/README_INTERFACE_TESTS.md +++ b/README_INTERFACE_TESTS.md @@ -89,29 +89,32 @@ ingress: - v1: - provider: - - - schema OK + - schema OK # schema found and valid - charms: - traefik-k8s (https://github.com/canonical/traefik-k8s-operator) custom_test_setup=no - requirer: - - - schema OK + - schema OK # schema found and valid - ``` ### Add interface tests -Create a python file at `/interfaces/ingress/v0/interface_tests/my_tests.py` with this content: +An interface test is any function named `test_*` that we can find in either file: +- `/interfaces/ingress/v0/interface_tests/test_provider.py` +- `/interfaces/ingress/v0/interface_tests/test_requirer.py` + +The name is important! + +Create a python file at `/interfaces/ingress/v0/interface_tests/test_requirer.py` with this content: ```python from scenario import State -from interface_tester.interface_test import interface_test_case - +from interface_tester import Tester -@interface_test_case( - event='ingress-relation-joined', - role='requirer', -) -def test_data_published_on_joined(output_state: State): - return +def test_data_published_on_joined(): + t = Tester(State()) + t.run("ingress-relation-joined") + t.assert_relation_data_empty() ``` Verify that the tests are specified correctly: @@ -131,7 +134,7 @@ ingress: - charms: - traefik-k8s (https://github.com/canonical/traefik-k8s-operator) custom_test_setup=no - requirer: - - test_data_published_on_joined:: ingress-relation-joined (state=no, schema=SchemaConfig.default) + - test_data_published_on_joined - schema OK - ``` @@ -168,15 +171,15 @@ The test would then become: ```python from scenario import State -from interface_tester.interface_test import interface_test_case +from interface_tester import Tester -@interface_test_case( - event='ingress-relation-joined', - role='requirer', -) -def test_data_published_on_joined(output_state: State): - assert output_state.status.unit.name == 'active' +def test_data_published_on_joined(): + t = Tester() + state_out: State = t.run("ingress-relation-joined") + t.assert_relation_data_empty() + + assert state_out.unit_status.name == 'active' ``` @@ -191,33 +194,31 @@ This becomes two test cases: ```python from scenario import State, Relation -from interface_tester.interface_test import interface_test_case, SchemaConfig +from interface_tester import Tester -@interface_test_case( - event='ingress-relation-joined', - role='provider', - input_state=State( +def test_data_published_on_joined_if_remote_has_sent_valid_data(output_state: State): + """If the requirer has provided correct data, then the provider will populate its side of the databag.""" + + t = Tester(State( relations=[Relation( endpoint='foo', interface='ingress', - remote_app_name='remote', # this is our simulated requirer + remote_app_name='remote', remote_app_data={ 'data': 'foo-bar', 'baz': 'qux' } )] - ) -) -def test_data_published_on_joined_if_remote_has_sent_valid_data(output_state: State): - """If the requirer has provided correct data, then the provider will populate its side of the databag.""" - + )) + state_out: State = t.run("ingress-relation-joined") + t.assert_schema_valid() -@interface_test_case( - event='ingress-relation-joined', - role='provider', - schema=SchemaConfig.empty, - input_state=State( + assert state_out.unit_status.name == 'blocked' + +def test_no_data_published_on_joined_if_remote_has_not_sent_valid_data(): + """If the requirer has provided INcorrect data, then the provider will not write anything to its databags.""" + t = Tester(State( relations=[Relation( endpoint='foo', interface='ingress', @@ -226,23 +227,25 @@ def test_data_published_on_joined_if_remote_has_sent_valid_data(output_state: St 'some': 'rubbish' } )] - ) -) -def test_no_data_published_on_joined_if_remote_has_not_sent_valid_data(output_state: State): - """If the requirer has provided INcorrect data, then the provider will not write anything to its databags.""" + )) + state_out: State = t.run("ingress-relation-joined") + t.assert_relation_data_empty() + + assert state_out.unit_status.name == 'blocked' + ``` -Note the usage of `SchemaConfig.empty`. That is what disables the 'default' schema validation and instructs the test runner to verify that the provider-side databags are empty instead of containing whatever they should contain according to `schema.py`. +Note the usage of `assert_relation_data_empty/assert_schema_valid`. Within the scope of an interface test you must call one of the two (or disable schema checking altogether with `skip_schema_validation`). # Reference: how does it work? Each interface test maps to a [Scenario test](https://github.com/canonical/ops-scenario). -The metadata passed to `interface_test_case`, along with metadata gathered from the charm being tested, is used to assemble a scenario test. Once that is done, each interface test can be broken down in three steps, each one verifying separate things in order: +The arguments passed to `Tester`, along with metadata gathered from the charm being tested, are used to assemble a scenario test. Once that is done, each interface test can be broken down in three steps, each one verifying separate things in order: - verify that the scenario test runs (i.e. it can produce an output state without the charm raising exceptions) -- verify that the output state is valid (by the interface-test-writer's definition) -- validate the local relation databags against the role's relation schema provided in `schema.py` +- verify that the output state is valid (by the interface-test-writer's definition): i.e. that the test function returns without raising any exception +- validate the local relation databags against the role's relation schema provided in `schema.py` (or against a custom schema) If any of these steps fail, the test as a whole is considered failed. @@ -264,20 +267,42 @@ If it says `NOT OK`, there is an error in the schema format or the filename. ### Referencing the schema in an interface test When you write an interface test for `ingress`, by default, the test case will validate the relation against the schema provider in `schema.py` (using the appropriate role). +In more complex cases, e.g. if the schema can assume one of multiple shapes depending on the position in a sequence of data exchanges, it can be handy to override that default. -`interface_tester.interface_test_case` accepts a `schema` argument that allows you to configure this behaviour. It can take one of four possible values: -- `interface_tester.interface_test.SchemaConfig.default` (or the string `"default"`): validate any `ingress` relation found in the `state_out` against the schema found in `schema.py`. If this interface test case is for the requirer, it will use the `RequirerSchema`; otherwise the `ProviderSchema`. -- `interface_tester.interface_test.SchemaConfig.skip` (or the string`"skip"`): skip schema validation for this test. -- `interface_tester.interface_test.SchemaConfig.empty` (or the string`"empty"`): assert that any `ingress` relation found in the `state_out` has **no relation data** at all (local side). -- you can pass a custom `interface_tester.schema_base.DataBagSchema` subclass, which will be used to validate any `ingress` relation found in `state_out`. This will replace the default one found in `schema.py` for this test only. - +`Tester.assert_schema_valid` accepts a `schema` argument that allows you to configure the expected schema. +Pass to it a custom `interface_tester.DataBagSchema` subclass and that will replace the default schema for this test. # Matrix-testing interface compliance If we have: - a `../interfaces/ingress/v0/charms.yaml` listing some providers and some requirers of the `ingress` interface. - a `../interfaces/ingress/v0/schema.py` specifying the interface schema (optional: schema validation will be skipped if not found) -- a `../interfaces/ingress/v0/interface_tests/my_tests.py` providing a list of interface tests for either role +- two `../interfaces/ingress/v0/interface_tests/test_[requirer|provider].py` files providing a list of interface tests for either role You can then run `python ./run_matrix.py ingress`. This will attempt to run the interface tests on all charms in `.../interfaces/ingress/v0/charms.yaml`. Omitting the `ingress` argument will run the tests for all interfaces (warning: might take some time.) + +# Charm repo configuration +When developing the tests, it can be useful to run them against a specific branch of a charm repo. To do that, write in `charms.yaml`: + +```yaml +providers: + - name: traefik-k8s + url: https://github.com/canonical/traefik-k8s-operator + branch: develop # any custom branch + +requirers: [] +``` + +Also it can be useful to configure where, relative to the repo root, the tester fixture can be found and what it is called. + +```yaml +providers: + - name: traefik-k8s + url: https://github.com/canonical/traefik-k8s-operator + test_setup: + - location: foo/bar/baz.py # location of the identifier + identifier: qux # name of a pytest fixture yielding a configured InterfaceTester + +requirers: [] +``` diff --git a/interfaces/ingress/v1/charms.yaml b/interfaces/ingress/v1/charms.yaml index 4351ce95..7c9568b3 100644 --- a/interfaces/ingress/v1/charms.yaml +++ b/interfaces/ingress/v1/charms.yaml @@ -1,5 +1,6 @@ providers: - name: traefik-k8s url: https://github.com/canonical/traefik-k8s-operator + branch: scenario-5.0 # todo: remove when https://github.com/canonical/traefik-k8s-operator/pull/239 lands requirers: [] \ No newline at end of file diff --git a/interfaces/ingress/v1/interface_tests/provider_tests.py b/interfaces/ingress/v1/interface_tests/provider_tests.py deleted file mode 100644 index 01bd54f7..00000000 --- a/interfaces/ingress/v1/interface_tests/provider_tests.py +++ /dev/null @@ -1,69 +0,0 @@ -# Copyright 2023 Canonical -# See LICENSE file for licensing details. -import yaml - -from interface_tester.interface_test import interface_test_case, SchemaConfig -from scenario import State, Relation - - -@interface_test_case( - event='ingress-relation-created', - role='provider', - schema=SchemaConfig.empty -) -def test_no_data_on_created(output_state: State): - # nothing happens on created: databags are empty - return - - -@interface_test_case( - event='ingress-relation-joined', - role='provider', - schema=SchemaConfig.empty -) -def test_no_data_on_joined(output_state: State): - # nothing happens on joined: databags are empty - return - - -@interface_test_case( - event='ingress-relation-changed', - role='provider', - input_state=State( - relations=[Relation( - # todo: endpoint is unknown/overwritten: find an elegant way to omit it here. - # perhaps input state is too general: we only need this relation's db contents: - endpoint='ingress', - interface='ingress', - remote_app_name='remote', - remote_app_data={ - 'host': '0.0.0.42', - 'model': 'bar', - 'name': 'remote/0', - 'port': '42' - } - )] - ) -) -def test_data_published_on_changed_remote_valid(output_state: State): - return # schema validation is enough for now - - -@interface_test_case( - event='ingress-relation-changed', - role='provider', - input_state=State(relations=[Relation( - endpoint='ingress', - interface='ingress', - remote_app_name='remote', - remote_app_data={ - 'port': '42', - 'bubble': 'rubble' - } - )] - ), - schema=SchemaConfig.empty -) -def test_data_published_on_changed_remote_invalid(output_state: State): - # on changed, if the remote side has sent INvalid data: local side didn't publish anything either. - return diff --git a/interfaces/ingress/v1/interface_tests/test_provider.py b/interfaces/ingress/v1/interface_tests/test_provider.py new file mode 100644 index 00000000..56392545 --- /dev/null +++ b/interfaces/ingress/v1/interface_tests/test_provider.py @@ -0,0 +1,74 @@ +# Copyright 2023 Canonical +# See LICENSE file for licensing details. +import yaml + +from interface_tester import Tester +from scenario import State, Relation + + +def test_no_data_on_created(): + t = Tester( + State( + relations=[ + Relation( + endpoint="ingress", + interface="ingress", + ) + ] + ) + ) + state_out = t.run("ingress-relation-created") + t.assert_relation_data_empty() + + +def test_no_data_on_joined(output_state: State): + # nothing happens on joined: databags are empty + t = Tester( + State( + relations=[ + Relation( + endpoint="ingress", + interface="ingress", + ) + ] + ) + ) + state_out = t.run("ingress-relation-joined") + + +def test_data_published_on_changed_remote_valid(output_state: State): + t = Tester( + State( + relations=[Relation( + endpoint='ingress', + interface='ingress', + remote_app_data={ + 'host': '0.0.0.42', + 'model': 'bar', + 'name': 'remote/0', + 'port': '42' + } + )] + ) + ) + state_out = t.run("ingress-relation-created") + t.assert_schema_valid() + + +def test_no_data_published_on_changed_remote_invalid(output_state: State): + # on changed, if the remote side has sent INvalid data: local side didn't publish anything either. + t = Tester( + State( + relations=[Relation( + endpoint='ingress', + interface='ingress', + remote_app_data={ + 'host': '0.0.0.42', + 'bubble': "10", + 'rubble': "foo" + } + )] + ) + ) + state_out = t.run("ingress-relation-created") + t.assert_relation_data_empty() diff --git a/interfaces/openfga/v0/interface_tests/provider_tests.py b/interfaces/openfga/v0/interface_tests/provider_tests.py deleted file mode 100644 index 4f30e921..00000000 --- a/interfaces/openfga/v0/interface_tests/provider_tests.py +++ /dev/null @@ -1,43 +0,0 @@ -# Copyright 2023 Canonical -# See LICENSE file for licensing details. -import yaml -from interface_tester.interface_test import SchemaConfig, interface_test_case -from scenario import Relation, State - - -@interface_test_case( - event='openfga-relation-created', - role='provider', - schema=SchemaConfig.empty -) -def test_no_data_on_created(output_state: State): - # nothing happens on created: databags are empty - return - - -@interface_test_case( - event='openfga-relation-joined', - role='provider', - schema=SchemaConfig.empty -) -def test_no_data_on_joined(output_state: State): - # nothing happens on joined: databags are empty - return - - -@interface_test_case( - event='openfga-relation-changed', - role='provider', - input_state=State( - relations=[Relation( - endpoint='openfga', - interface='openfga', - remote_app_name='remote', - remote_app_data={ - 'store_name': 'test-store' - } - )] - ) -) -def test_data_published_on_changed_remote_valid(output_state: State): - return # schema validation is enough for now diff --git a/interfaces/openfga/v0/interface_tests/test_provider.py b/interfaces/openfga/v0/interface_tests/test_provider.py new file mode 100644 index 00000000..ad5ec123 --- /dev/null +++ b/interfaces/openfga/v0/interface_tests/test_provider.py @@ -0,0 +1,34 @@ +# Copyright 2023 Canonical +# See LICENSE file for licensing details. +from interface_tester import Tester +from scenario import Relation, State + + +def test_no_data_on_created(): + # nothing happens on created: databags are empty + t = Tester() + t.run('openfga-relation-created') + t.assert_relation_data_empty() + + +def test_no_data_on_joined(): + # nothing happens on joined: databags are empty + t = Tester() + t.run('openfga-relation-joined') + t.assert_relation_data_empty() + + +def test_data_published_on_changed_remote_valid(): + t = Tester(State( + relations=[Relation( + endpoint='openfga', + interface='openfga', + remote_app_name='remote', + remote_app_data={ + 'store_name': 'test-store' + } + )] + )) + t.run('openfga-relation-changed') + t.assert_schema_valid() + diff --git a/interfaces/openfga/v0/interface_tests/requirer_tests.py b/interfaces/openfga/v0/interface_tests/test_requirer.py similarity index 61% rename from interfaces/openfga/v0/interface_tests/requirer_tests.py rename to interfaces/openfga/v0/interface_tests/test_requirer.py index d994cf4b..1ba45866 100644 --- a/interfaces/openfga/v0/interface_tests/requirer_tests.py +++ b/interfaces/openfga/v0/interface_tests/test_requirer.py @@ -1,34 +1,25 @@ # Copyright 2023 Canonical # See LICENSE file for licensing details. -import yaml -from interface_tester.interface_test import SchemaConfig, interface_test_case +from interface_tester import Tester from scenario import Relation, State -@interface_test_case( - event='openfga-relation-created', - role='requirer', - schema=SchemaConfig.empty -) def test_no_data_on_created(output_state: State): # nothing happens on created: databags are empty - return + t = Tester() + t.run('openfga-relation-created') + t.assert_relation_data_empty() -@interface_test_case( - event='openfga-relation-joined', - role='requirer', - schema=SchemaConfig.empty -) def test_no_data_on_joined(output_state: State): # nothing happens on joined: databags are empty - return + t = Tester() + t.run('openfga-relation-joined') + t.assert_relation_data_empty() -@interface_test_case( - event='openfga-relation-changed', - role='requirer', - input_state=State( +def test_data_published_on_changed_remote_valid(output_state: State): + t = Tester(State( relations=[Relation( endpoint='openfga', interface='openfga', @@ -41,7 +32,6 @@ def test_no_data_on_joined(output_state: State): 'store_id': '01GK13VYZK62Q1T0X55Q2BHYD6', } )] - ) -) -def test_data_published_on_changed_remote_valid(output_state: State): - return # schema validation is enough for now + )) + t.run('openfga-relation-changed') + t.assert_schema_valid() diff --git a/pyproject.toml b/pyproject.toml index 3aed878f..9ac3731e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,20 +34,20 @@ classifiers = [ [project.optional-dependencies] unit_tests = [ - "ops==2.2.0", - "PyYAML==6.0", - "pytest==7.3.1", - "ops-scenario==2.2", + "ops>=2.2.0", + "PyYAML>=6.0", + "pytest>=7.3.1", + "ops-scenario>=5", ] json_schemas = [ - "PyYAML==6.0", + "PyYAML>=6.0", ] interface_tests = [ - "PyYAML==6.0", - "pytest==7.3.1", - "ops-scenario==2.2", + "PyYAML>=6.0", + "pytest>=7.3.1", + "ops-scenario>=5", "pydantic==1.10.7", "requests==2.28.1", "virtualenv==20.21.0" diff --git a/run_matrix.py b/run_matrix.py index 7595b6df..33d800d5 100644 --- a/run_matrix.py +++ b/run_matrix.py @@ -148,7 +148,7 @@ def _setup_venv(charm_path: Path) -> None: logging.info(f"Installing dependencies in venv for {charm_path}") subprocess.check_call( - ".interface-venv/bin/python -m pip install setuptools pytest interface-tester-pytest", + ".interface-venv/bin/python -m pip install setuptools pytest pytest-interface-tester", shell=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, From 5044973853d22b146a31a6db5b2c1ba1708f8f20 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Wed, 6 Sep 2023 14:03:38 +0200 Subject: [PATCH 2/9] dep vbump --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9ac3731e..328cd510 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ keywords = ["juju", "relation interfaces"] dependencies = [ "pydantic==1.10.7", - "pytest-interface-tester==0.1.1" + "pytest-interface-tester>=1.0" ] readme = "README.md" From 48c017b65b125d03ca68a6262cafcdaaa5990d0c Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Thu, 14 Sep 2023 14:35:07 +0200 Subject: [PATCH 3/9] fixed schema exporter --- docs/json_schemas/ingress/v0/provider.json | 51 ---------------------- docs/json_schemas/ingress/v0/requirer.json | 51 ---------------------- interfaces/nfs_share/v0/schema.py | 2 +- 3 files changed, 1 insertion(+), 103 deletions(-) delete mode 100644 docs/json_schemas/ingress/v0/provider.json delete mode 100644 docs/json_schemas/ingress/v0/requirer.json diff --git a/docs/json_schemas/ingress/v0/provider.json b/docs/json_schemas/ingress/v0/provider.json deleted file mode 100644 index c9da0364..00000000 --- a/docs/json_schemas/ingress/v0/provider.json +++ /dev/null @@ -1,51 +0,0 @@ -{ - "title": "ProviderSchema", - "description": "Provider schema for Ingress.", - "type": "object", - "properties": { - "unit": { - "$ref": "#/definitions/BaseModel" - }, - "app": { - "$ref": "#/definitions/MyProviderData" - } - }, - "required": [ - "app" - ], - "definitions": { - "BaseModel": { - "title": "BaseModel", - "type": "object", - "properties": {} - }, - "Url": { - "title": "Url", - "type": "object", - "properties": { - "url": { - "title": "Url", - "minLength": 1, - "maxLength": 65536, - "format": "uri", - "type": "string" - } - }, - "required": [ - "url" - ] - }, - "MyProviderData": { - "title": "MyProviderData", - "type": "object", - "properties": { - "ingress": { - "$ref": "#/definitions/Url" - } - }, - "required": [ - "ingress" - ] - } - } -} \ No newline at end of file diff --git a/docs/json_schemas/ingress/v0/requirer.json b/docs/json_schemas/ingress/v0/requirer.json deleted file mode 100644 index 9bd71d60..00000000 --- a/docs/json_schemas/ingress/v0/requirer.json +++ /dev/null @@ -1,51 +0,0 @@ -{ - "title": "RequirerSchema", - "description": "Requirer schema for Ingress.", - "type": "object", - "properties": { - "unit": { - "$ref": "#/definitions/BaseModel" - }, - "app": { - "$ref": "#/definitions/IngressRequirerData" - } - }, - "required": [ - "app" - ], - "definitions": { - "BaseModel": { - "title": "BaseModel", - "type": "object", - "properties": {} - }, - "IngressRequirerData": { - "title": "IngressRequirerData", - "type": "object", - "properties": { - "port": { - "title": "Port", - "type": "string" - }, - "host": { - "title": "Host", - "type": "string" - }, - "model": { - "title": "Model", - "type": "string" - }, - "name": { - "title": "Name", - "type": "string" - } - }, - "required": [ - "port", - "host", - "model", - "name" - ] - } - } -} \ No newline at end of file diff --git a/interfaces/nfs_share/v0/schema.py b/interfaces/nfs_share/v0/schema.py index d5af302b..d9dd989b 100644 --- a/interfaces/nfs_share/v0/schema.py +++ b/interfaces/nfs_share/v0/schema.py @@ -23,7 +23,7 @@ from pydantic import BaseModel -from interfaces.schema_base import DataBagSchema +from interface_tester.schema_base import DataBagSchema class NFSShareProviderAppData(BaseModel): From d69c9d39bf9217740dbd893862904b21270e6761 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Thu, 14 Sep 2023 15:32:59 +0200 Subject: [PATCH 4/9] tests green --- interfaces/ingress/v1/charms.yaml | 1 - .../v1/interface_tests/test_provider.py | 54 ++++++++----------- pyproject.toml | 2 +- run_matrix.py | 26 +++++---- tests/test_unit/test_build.py | 6 +-- tests/test_unit/test_interface_tests.py | 13 +++-- 6 files changed, 50 insertions(+), 52 deletions(-) diff --git a/interfaces/ingress/v1/charms.yaml b/interfaces/ingress/v1/charms.yaml index 7c9568b3..4351ce95 100644 --- a/interfaces/ingress/v1/charms.yaml +++ b/interfaces/ingress/v1/charms.yaml @@ -1,6 +1,5 @@ providers: - name: traefik-k8s url: https://github.com/canonical/traefik-k8s-operator - branch: scenario-5.0 # todo: remove when https://github.com/canonical/traefik-k8s-operator/pull/239 lands requirers: [] \ No newline at end of file diff --git a/interfaces/ingress/v1/interface_tests/test_provider.py b/interfaces/ingress/v1/interface_tests/test_provider.py index 56392545..b38ed0fb 100644 --- a/interfaces/ingress/v1/interface_tests/test_provider.py +++ b/interfaces/ingress/v1/interface_tests/test_provider.py @@ -8,7 +8,7 @@ def test_no_data_on_created(): t = Tester( - State( + State(leader=True, relations=[ Relation( endpoint="ingress", @@ -21,10 +21,10 @@ def test_no_data_on_created(): t.assert_relation_data_empty() -def test_no_data_on_joined(output_state: State): +def test_no_data_on_joined(): # nothing happens on joined: databags are empty t = Tester( - State( + State(leader=True, relations=[ Relation( endpoint="ingress", @@ -34,41 +34,33 @@ def test_no_data_on_joined(output_state: State): ) ) state_out = t.run("ingress-relation-joined") + t.assert_relation_data_empty() -def test_data_published_on_changed_remote_valid(output_state: State): - t = Tester( - State( - relations=[Relation( - endpoint='ingress', - interface='ingress', - remote_app_data={ - 'host': '0.0.0.42', - 'model': 'bar', - 'name': 'remote/0', - 'port': '42' - } - )] - ) +def test_data_published_on_changed_remote_valid(): + ingress = Relation( + endpoint='ingress', interface='ingress', + remote_app_data={'host': '"0.0.0.42"', 'model': '"bar"', 'name': '"remote/0"', 'port': '42'} ) - state_out = t.run("ingress-relation-created") + t = Tester(State(leader=True, relations=[ingress])) + state_out = t.run(ingress.changed_event) t.assert_schema_valid() -def test_no_data_published_on_changed_remote_invalid(output_state: State): +def test_no_data_published_on_changed_remote_invalid(): # on changed, if the remote side has sent INvalid data: local side didn't publish anything either. t = Tester( - State( - relations=[Relation( - endpoint='ingress', - interface='ingress', - remote_app_data={ - 'host': '0.0.0.42', - 'bubble': "10", - 'rubble': "foo" - } - )] - ) + State(leader=True, + relations=[Relation( + endpoint='ingress', + interface='ingress', + remote_app_data={ + 'host': '0.0.0.42', + 'bubble': "10", + 'rubble': "foo" + } + )] + ) ) - state_out = t.run("ingress-relation-created") + state_out = t.run("ingress-relation-changed") t.assert_relation_data_empty() diff --git a/pyproject.toml b/pyproject.toml index 328cd510..a4444fff 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,7 @@ keywords = ["juju", "relation interfaces"] dependencies = [ "pydantic==1.10.7", - "pytest-interface-tester>=1.0" + "pytest-interface-tester>=1.0.2" ] readme = "README.md" diff --git a/run_matrix.py b/run_matrix.py index 33d800d5..144aa0fe 100644 --- a/run_matrix.py +++ b/run_matrix.py @@ -57,18 +57,23 @@ def _clone_charm_repo(charm_config: "_CharmTestConfig", charm_path: Path): f"custom branch provided for {charm_config.name}; " f"this should only be done in staging" ) - subprocess.call( + retcode = subprocess.call( f"git clone --quiet --depth 1 {branch_option} {charm_config.url} {charm_path}", shell=True, stdout=subprocess.DEVNULL, ) + if retcode > 0: + raise SetupError( + f"Failed to clone repo for {charm_config.name}; " + "check the charms.yaml config." + ) def _prepare_repo( - charm_config: "_CharmTestConfig", - interface: str, - version: int, - root: Path = Path("/tmp/charm-relation-interfaces-tests/"), + charm_config: "_CharmTestConfig", + interface: str, + version: int, + root: Path = Path("/tmp/charm-relation-interfaces-tests/"), ) -> Tuple[Path, Path]: """Clone the charm repository and create the venv if it hasn't been done already.""" logging.info(f"Preparing testing environment for: {charm_config.name}") @@ -109,7 +114,7 @@ def test_{interface}_interface({fixture_id}: InterfaceTester): def _generate_test( - interface: str, test_path: Path, fixture_id: str, version: int + interface: str, test_path: Path, fixture_id: str, version: int ) -> Path: """Generate a pytest file for a given charm and interface.""" logging.info(f"Generating test file for {interface} at {test_path}") @@ -180,7 +185,7 @@ def _run_test_with_pytest(root: Path, test_path: Path): def _test_charm( - charm_config: "_CharmTestConfig", interface: str, version: int, role: str + charm_config: "_CharmTestConfig", interface: str, version: int, role: str ) -> bool: """Run interface tests for a charm.""" logging.info(f"Running tests for charm: {charm_config.name}") @@ -205,7 +210,7 @@ def _test_charm( def _test_charms( - charm_configs: Iterable["_CharmTestConfig"], interface: str, version: int, role: str + charm_configs: Iterable["_CharmTestConfig"], interface: str, version: int, role: str ) -> "_ResultsPerCharm": """Test all charms against this interface and role.""" logging.info(f"Running tests for {interface}") @@ -218,7 +223,7 @@ def _test_charms( def _test_roles( - tests_per_role: Dict["_Role", "_RoleTestSpec"], interface: str, version: int + tests_per_role: Dict["_Role", "_RoleTestSpec"], interface: str, version: int ) -> "_ResultsPerRole": """Run the tests for each role of this interface.""" results_per_role: _ResultsPerRole = {} @@ -293,4 +298,5 @@ def pprint_interface_test_results(test_results: dict): ) args = parser.parse_args() - pprint_interface_test_results(run_interface_tests(Path("."), args.include)) + result = run_interface_tests(Path("."), args.include) + pprint_interface_test_results(result) diff --git a/tests/test_unit/test_build.py b/tests/test_unit/test_build.py index dd115e9d..56e60e80 100644 --- a/tests/test_unit/test_build.py +++ b/tests/test_unit/test_build.py @@ -180,7 +180,5 @@ def test_build_schemas_broken_source(tmp_path, source, caplog): output_location=pth, ) - assert ( - f"Found object called 'RequirerSchema' in {schema_path}; " - f"expecting a DataBagSchema subclass, not" in caplog.text - ) + assert (f"Found object called RequirerSchema in {schema_path}; " + f"expecting a DataBagSchema subclass, not ") in caplog.text diff --git a/tests/test_unit/test_interface_tests.py b/tests/test_unit/test_interface_tests.py index 09498093..e8a6b4ff 100644 --- a/tests/test_unit/test_interface_tests.py +++ b/tests/test_unit/test_interface_tests.py @@ -1,3 +1,4 @@ +import json from pathlib import Path import pytest @@ -21,7 +22,9 @@ def __init__(self, framework: Framework): self.framework.observe(self.on.ingress_relation_changed, self._on_changed) def _on_changed(self, e): - if e.relation.data[e.relation.app].get("host"): + appdata = e.relation.data[e.relation.app] + # simplification of valid ingress requirer data + if all(appdata.get(key) for key in ("host", "port", "model", "name")): url = yaml.safe_dump({"url": "http://foo.com"}) e.relation.data[self.app]["ingress"] = url else: @@ -38,10 +41,10 @@ def __init__(self, framework: Framework): def _on_created(self, e): if self.unit.is_leader(): data = { - "host": "foo", - "port": "10", - "model": "baz", - "name": self.unit.name, + "host": '"foo"', + "port": '10', + "model": '"baz"', + "name": json.dumps(self.unit.name), } e.relation.data[self.app]["data"] = yaml.safe_dump(data) From 55b862ad03c8491f5b0e92dc97acfa00b41605ac Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Thu, 14 Sep 2023 15:41:00 +0200 Subject: [PATCH 5/9] bumped ops dep --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 55999b42..927b21b6 100644 --- a/tox.ini +++ b/tox.ini @@ -63,7 +63,7 @@ commands = [testenv:build-json-schemas] description = build json schemas in docs/ deps = - ops==2.2.0 # https://github.com/canonical/ops-scenario/issues/48 + ops==2.6.0 .[json_schemas] setenv = PYTHONPATH={toxinidir} From 2c6a92f758fdf88e23ac5ab3c6bc353dc88fc6c7 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Thu, 14 Sep 2023 15:43:49 +0200 Subject: [PATCH 6/9] lint and unpinned ops v in discover CI --- .github/workflows/discover.yaml | 3 +-- run_matrix.py | 16 ++++++++-------- tests/test_unit/test_build.py | 6 ++++-- tests/test_unit/test_interface_tests.py | 2 +- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/.github/workflows/discover.yaml b/.github/workflows/discover.yaml index f5bef1d1..b3802d6f 100644 --- a/.github/workflows/discover.yaml +++ b/.github/workflows/discover.yaml @@ -18,7 +18,6 @@ jobs: python-version: 3.8 - name: install interface-tester run: | - # TODO remove "ops<2.5": https://github.com/canonical/ops-scenario/issues/48 - python -m pip install pytest-interface-tester "ops<2.5" + python -m pip install pytest-interface-tester ops - name: run discover run: interface_tester discover diff --git a/run_matrix.py b/run_matrix.py index 144aa0fe..df06c776 100644 --- a/run_matrix.py +++ b/run_matrix.py @@ -70,10 +70,10 @@ def _clone_charm_repo(charm_config: "_CharmTestConfig", charm_path: Path): def _prepare_repo( - charm_config: "_CharmTestConfig", - interface: str, - version: int, - root: Path = Path("/tmp/charm-relation-interfaces-tests/"), + charm_config: "_CharmTestConfig", + interface: str, + version: int, + root: Path = Path("/tmp/charm-relation-interfaces-tests/"), ) -> Tuple[Path, Path]: """Clone the charm repository and create the venv if it hasn't been done already.""" logging.info(f"Preparing testing environment for: {charm_config.name}") @@ -114,7 +114,7 @@ def test_{interface}_interface({fixture_id}: InterfaceTester): def _generate_test( - interface: str, test_path: Path, fixture_id: str, version: int + interface: str, test_path: Path, fixture_id: str, version: int ) -> Path: """Generate a pytest file for a given charm and interface.""" logging.info(f"Generating test file for {interface} at {test_path}") @@ -185,7 +185,7 @@ def _run_test_with_pytest(root: Path, test_path: Path): def _test_charm( - charm_config: "_CharmTestConfig", interface: str, version: int, role: str + charm_config: "_CharmTestConfig", interface: str, version: int, role: str ) -> bool: """Run interface tests for a charm.""" logging.info(f"Running tests for charm: {charm_config.name}") @@ -210,7 +210,7 @@ def _test_charm( def _test_charms( - charm_configs: Iterable["_CharmTestConfig"], interface: str, version: int, role: str + charm_configs: Iterable["_CharmTestConfig"], interface: str, version: int, role: str ) -> "_ResultsPerCharm": """Test all charms against this interface and role.""" logging.info(f"Running tests for {interface}") @@ -223,7 +223,7 @@ def _test_charms( def _test_roles( - tests_per_role: Dict["_Role", "_RoleTestSpec"], interface: str, version: int + tests_per_role: Dict["_Role", "_RoleTestSpec"], interface: str, version: int ) -> "_ResultsPerRole": """Run the tests for each role of this interface.""" results_per_role: _ResultsPerRole = {} diff --git a/tests/test_unit/test_build.py b/tests/test_unit/test_build.py index 56e60e80..b81ebc10 100644 --- a/tests/test_unit/test_build.py +++ b/tests/test_unit/test_build.py @@ -180,5 +180,7 @@ def test_build_schemas_broken_source(tmp_path, source, caplog): output_location=pth, ) - assert (f"Found object called RequirerSchema in {schema_path}; " - f"expecting a DataBagSchema subclass, not ") in caplog.text + assert ( + f"Found object called RequirerSchema in {schema_path}; " + f"expecting a DataBagSchema subclass, not " + ) in caplog.text diff --git a/tests/test_unit/test_interface_tests.py b/tests/test_unit/test_interface_tests.py index e8a6b4ff..c4a63cf9 100644 --- a/tests/test_unit/test_interface_tests.py +++ b/tests/test_unit/test_interface_tests.py @@ -42,7 +42,7 @@ def _on_created(self, e): if self.unit.is_leader(): data = { "host": '"foo"', - "port": '10', + "port": "10", "model": '"baz"', "name": json.dumps(self.unit.name), } From 0b624ee48b30d028a44c8d81b17bbd50c29cd331 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Thu, 14 Sep 2023 16:55:16 +0200 Subject: [PATCH 7/9] added charms yaml file format doc --- CHARMS_YAML_SPEC.md | 36 ++++++++++++++++++++++++++++++++++++ README_INTERFACE_TESTS.md | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 CHARMS_YAML_SPEC.md diff --git a/CHARMS_YAML_SPEC.md b/CHARMS_YAML_SPEC.md new file mode 100644 index 00000000..f6fe45db --- /dev/null +++ b/CHARMS_YAML_SPEC.md @@ -0,0 +1,36 @@ +Each version of each interface listed in this repository is expected to contain a `charms.yaml` file, listing the reference charms implementing it. + +The format of the file is: + +```yaml +# (Mandatory) List of requirer charms (and optionally their test configs) +requirers: + + # (Mandatory) name of the charm, same as what is listed in its charmcraft.yaml + # example: grafana-k8s + - name: + + # (Mandatory) url of a git repo at which the charm source code can be found + # example: https://github.com/canonical/grafana-k8s-operator + url: + + # (Optional): test configuration + test_setup: + + # (Optional) + # name of a pytest fixture (a function) **yielding** a configured + # `interface_tester.InterfaceTester` object. + # default: "interface_tester" + identifier: + + # (Optional) path to the (conftest.py) python file containing the identifier + # called . Path is relative to the root of + # the charm repository as specified in `url` above. + # default: tests/interface/conftest.py + location: path/to/file.py + +# (Mandatory) List of provider charms (and optionally their test configs) +providers: [] # format is same as `requirers` +``` + + diff --git a/README_INTERFACE_TESTS.md b/README_INTERFACE_TESTS.md index a46b47cd..a397312e 100644 --- a/README_INTERFACE_TESTS.md +++ b/README_INTERFACE_TESTS.md @@ -29,7 +29,7 @@ providers: requirers: [] ``` -Verify that the `charms.yaml` file format is correct: +Verify that the [`charms.yaml` file format](CHARMS_YAML_SPEC.md) is correct: `interface_tester discover --include ingress` From dcbe510a4770a18d11bb5837d52519db1b2ab786 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Thu, 14 Sep 2023 17:02:27 +0200 Subject: [PATCH 8/9] more details and rename --- CHARMS_YAML_SPEC.md => README_CHARMS_YAML.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) rename CHARMS_YAML_SPEC.md => README_CHARMS_YAML.md (68%) diff --git a/CHARMS_YAML_SPEC.md b/README_CHARMS_YAML.md similarity index 68% rename from CHARMS_YAML_SPEC.md rename to README_CHARMS_YAML.md index f6fe45db..29586d01 100644 --- a/CHARMS_YAML_SPEC.md +++ b/README_CHARMS_YAML.md @@ -14,7 +14,14 @@ requirers: # example: https://github.com/canonical/grafana-k8s-operator url: - # (Optional): test configuration + # (Optional): Configuration for the test runner. It tells the interface test + # runner how to set up the context for the charm to be testable under the + # conditions required by this repository. Necessary if the charm requires, + # for example, a specific config, leadership, container connectivity, etc... in + # order to process the relation events as requested in order to comply with + # the interface. + # For more details, see the interface-tester-pytest documentation at: + # https://github.com/canonical/interface-tester-pytest test_setup: # (Optional) @@ -31,6 +38,4 @@ requirers: # (Mandatory) List of provider charms (and optionally their test configs) providers: [] # format is same as `requirers` -``` - - +``` \ No newline at end of file From c0c3fc6c7aef7eab02763de3c18504365fdaaa1e Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Thu, 14 Sep 2023 17:02:57 +0200 Subject: [PATCH 9/9] rename 2 --- README_INTERFACE_TESTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README_INTERFACE_TESTS.md b/README_INTERFACE_TESTS.md index a397312e..c37385b5 100644 --- a/README_INTERFACE_TESTS.md +++ b/README_INTERFACE_TESTS.md @@ -29,7 +29,7 @@ providers: requirers: [] ``` -Verify that the [`charms.yaml` file format](CHARMS_YAML_SPEC.md) is correct: +Verify that the [`charms.yaml` file format](README_CHARMS_YAML.md) is correct: `interface_tester discover --include ingress`