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.

Sign up for GitHub

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

Jump to bottom

[uss_qualifier] oir_simple: check subscription extent needs to cover OIR #803

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+366 −13
Open

[uss_qualifier] oir_simple: check subscription extent needs to cover OIR #803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c47227
[uss_qualifier] oir_simple: check subscription extent needs to cover OIR
Shastick Oct 11, 2024
712fda5
first pass of comments
Shastick Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion monitoring/prober/infrastructure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def wrapper_default_scope(*args, **kwargs):
resource_type_code_descriptions: Dict[ResourceType, str] = {}


# Next code: 398
# Next code: 400
def register_resource_type(code: int, description: str) -> ResourceType:
"""Register that the specified code refers to the described resource.

Expand Down
10 changes: 10 additions & 0 deletions ...qualifier/scenarios/astm/utm/dss/fragments/oir/oir_has_expected_subscription.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# OIR is attached to expected subscription test step fragment

## [Query Success](./crud/read_query.md)

Check query succeeds

## 🛑 OIR is attached to expected subscription check

If the OIR returned by the DSS under test is not attached to the expected subscription,
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../../requirements/astm/f3548/v21.md)**
15 changes: 15 additions & 0 deletions ...g/uss_qualifier/scenarios/astm/utm/dss/fragments/oir/oir_subscription_update.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# OIR's subscription can be updated test step fragment

## [Update the OIR's subscription](./crud/update_query.md)

This step verifies that an OIR attached to an explicit subscription can be mutated in order to be attached
to another explicit subscription that properly covers the extent of the OIR.

## [Fetch the OIR](./crud/read_query.md)

To determine if the OIR is attached to the correct subscription, the OIR is directly fetched from the DSS.

## 🛑 OIR is attached to expected subscription check

If the OIR returned by the DSS under test is not attached to the expected subscription,
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../../../requirements/astm/f3548/v21.md)**
60 changes: 58 additions & 2 deletions monitoring/uss_qualifier/scenarios/astm/utm/dss/op_intent_ref_simple.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,70 @@ Verifies the behavior of a DSS for simple interactions pertaining to operational

This step ensures that no entities with the known test IDs exists in the DSS.

### [Create an operational intent reference test step](./fragments/oir/crud/create_query.md)
### [Create a subscription test step](./fragments/sub/crud/create_query.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this new step not in the new case below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first case does not need to run that step, as it uses the subscription created in the setup case.

I tried to run the setup test case multiple times as part of some cleanup between test cases, but the logic won't allow to run it multiple times.

Possibly I could define a "reset workspace" test case instead and run it multiple times if that is allowed?

Copy link
Contributor

Choose a reason for hiding this comment

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

The first case does not need to run that step, as it uses the subscription created in the setup case.

Not sure I understand your response, given that my comment is on the subscription step.

That subscription is needed is all cases? Then OK for it to be in the setup. It's needed in first case only? Why not putting in the first case then?

And why running the setup multiple times would be needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The subscription is needed in some steps.

From our back and forth so far and your other comments, I expect that you would prefer a solution where:

  • the setup test case is limited to cleaning up entities that might have been left over from an earlier scenario, nothing else
  • each test case precisely creates and removes the entities it needs

Can you confirm this would work for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

the setup test case is limited to cleaning up entities that might have been left over from an earlier scenario, nothing else

Not necessarily: ground preparation for the rest of the scenario (without actually testing stuff relevant to the scenario) is fine, made for that even. There was just:

  • confusion on my end regarding to why this subscription is created here, naming it and specifying its properties help
  • intent to scope creation of things close to where they are needed, but they are needed in multiple cases so that works


Create an explicit subscription to be used in this scenario.

## Validate explicit subscription on OIR creation test case

Ensures that the explicit subscription provided upon creation of an OIR is properly validated and attached to the OIR.

### Provide subscription not covering extent of OIR being created test step

This step verifies that an OIR cannot be created when an explicit subscription that does not cover the extent of the OIR is specified.

#### 🛑 Request to create OIR with too short subscription fails check

If the DSS under test allows the qualifier to create an OIR with an explicit subscription that does not cover the extent of the OIR,
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../requirements/astm/f3548/v21.md)**

### [Create an OIR with correct explicit subscription test step](./fragments/oir/crud/create_query.md)

When the provided subscription covers the extent of the OIR, the OIR can be created.

#### [OIR is attached to expected subscription](./fragments/oir/oir_has_expected_subscription.md)

This step verifies that the OIR is attached to the subscription provided upon creation.

## Validate explicit subscription upon subscription replacement test case

Ensures that when the explicit subscription tied to an OIR is replaced with another explicit subscription,
this subscription is properly validated and attached to the OIR.

### [Create a subscription test step](./fragments/sub/crud/create_query.md)

Create an operational intent reference to be used in this scenario.
Create an additional explicit subscription to be used in this test case.

### Attempt to replace OIR's existing explicit subscription with an insufficient one test step

This step verifies that an OIR's existing explicit subscription cannot be replaced with an explicit subscription that does not cover the extent of the OIR.

#### 🛑 Request to mutate OIR while providing a too short subscription fails check

If the DSS under test allows the qualifier to replace an OIR's existing explicit subscription with an explicit subscription that does not cover the extent of the OIR,
it is in violation of **[astm.f3548.v21.DSS0005,1](../../../../requirements/astm/f3548/v21.md)**

#### [OIR is attached to expected subscription](./fragments/oir/oir_has_expected_subscription.md)

Verify that the OIR is still attached to the previous, valid, subscription.

### [Replace the OIR's explicit subscription test step](./fragments/oir/oir_subscription_update.md)

This step verifies that an OIR attached to an explicit subscription can be mutated in order to be attached
to another explicit subscription that properly covers the extent of the OIR.

## Deletion requires correct OVN test case

Ensures that a DSS will only delete OIRs when the correct OVN is presented.

### [Ensure clean workspace test step](./clean_workspace.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is a good idea to start including this 'clean workspace' step in the middle of scenarios. If some generic cleanup is required it may be a sign that a different scenario is required. Of that may not be required an just specific entities may need to be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention is mainly to reset entities to a clearly known state. Some thoughts here:

  • I don't think it is wise to split things to separate scenarios as soon as we have "cleanup" needs between test cases: scenarios such as OIRSimple may end up doing many variants of the same operation to check various corner cases, and separating each corner case to its own scenario seems counter-productive.
  • if test cases contain steps to do some resetting/cleanups, I think it is worth having those clearly documented, and a dedicated step seems ideal for that?

that may not be required an just specific entities may need to be removed.

One thing that could be done to have a more specific cleanup is to just remove the known entities without doing the search. It will still require its own step though, which is why I went with the option of just re-cleaning and re-setting the workspace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a sidenote: we already do such cleaning at the beginning of some test cases in the implicit subscription handling scenario, for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing that could be done to have a more specific cleanup is to just remove the known entities without doing the search.

Yeah that was my (poorly communicated) point. Let me rephrase it hopefully better.
During executions of test scenarios we are supposed to know exactly the expected state of the system. As such whenever we need to remove stuff in the middle of the scenario, we should be able to know exactly for what entities that is.
The blanket removal of entities in an area before/after the scenario is meant as a technical measure to avoid leakage of a test scenario into another, but not as the emulation of user behavior in the way the scenario's steps are.
So if cleanup is needed within a scenario, it should be targeted to remain aligned with that test scenario spirit. Even if that actually induces more steps in the scenario.
And in addition (but that's more in the realm of opinion), having to manually manage cleanups of entities should encourage us to keep scenarios minimal and targeted to what they are supposed to test, which is a good thing.


This step resets the workspace for the present and following test cases by ensuring that no entities with the known test IDs exists in the DSS.

### [Create an operational intent reference test step](./fragments/oir/crud/create_query.md)
mickmis marked this conversation as resolved.
Show resolved Hide resolved

Create the operational intent reference that will be used for the deletion attempts that happen in the subsequent steps.

### Attempt deletion with missing OVN test step

This step verifies that an existing OIR cannot be deleted with a missing OVN.
Expand Down
Loading
Loading