[uss_qualifier] oir_simple: check subscription extent needs to cover OIR #803
Conversation
Shastick
force-pushed
the
oir-simple-0-create-short-explicit-sub
branch
from
a0a0bfb
to
3a10cf3
Compare
| @@ -28,14 +28,74 @@ 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 a subscription test step](./fragments/sub/crud/create_query.md) | |||
There was a problem hiding this comment.
Why is this new step not in the new case below?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
|
|
||
| ## 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
As a sidenote: we already do such cleaning at the beginning of some test cases in the implicit subscription handling scenario, for example.
There was a problem hiding this comment.
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 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 incorrect subscription fails check |
There was a problem hiding this comment.
Can you be more specific than 'incorrect' in the check title?
| ### [Create an operational intent reference test step](./fragments/oir/crud/create_query.md) | ||
|
|
||
| Create an operational intent reference to be used in this scenario. | ||
| When the provided subscription covers the extent of the OIR, the OIR can be created. | ||
|
|
||
| ### [OIR is attached to expected subscription test step](./fragments/oir/oir_has_expected_subscription.md) | ||
|
|
||
| This step verifies that the OIR is attached to the subscription provided upon creation. |
There was a problem hiding this comment.
Looks like those two steps are actually a single one: include one of them as a fragment.
There was a problem hiding this comment.
Created a new oir_has_expected_subscription fragment which is now part of the "Create an operational intent reference" 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 an incorrect subscription fails check |
There was a problem hiding this comment.
Can you be more specific than 'incorrect' in the check title?
There was a problem hiding this comment.
replaced with 'too short'
| 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 test step](./fragments/oir/oir_has_expected_subscription.md) |
There was a problem hiding this comment.
Sounds like this shouldn't be a step but rather included as a fragment under the above step.
| 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. | ||
|
|
||
| ### [OIR is attached to expected subscription test step](./fragments/oir/oir_has_expected_subscription.md) |
There was a problem hiding this comment.
Sounds like this shouldn't be a step but rather included as a fragment under the above step.
Shastick
force-pushed
the
oir-simple-0-create-short-explicit-sub
branch
from
06f2e7c
to
712fda5
Compare
There was a problem hiding this comment.
TBH I'm having a hard time understanding the scenario and I end up spending too much time doing that. But I need to understand the intent of the test and determine if it tests what it claims to test to properly review it. That also means it will also be difficult for users to understand the scenario when they will use it.
Notably it is difficult to understand for what purpose and why entities are created, and what's special about them. Naming them and highlighting their properties would help. It would be great if you could keep it in mind as well for future scenarios as I expect that we will have to produce many of them in the short-term. Thanks!
|
|
||
| ## 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) |
There was a problem hiding this comment.
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.
| @@ -28,14 +28,74 @@ 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 a subscription test step](./fragments/sub/crud/create_query.md) | |||
There was a problem hiding this comment.
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?
Extend the OIRSimple scenario with two test cases that verify that the DSS properly validates explicit subscriptions being attached to an OIR upon creation or mutation.
(This covers situation 0 and 3 from this DSS issue)