[uss_qualifier/documentation] Split clean_workspace fragment into fragments for different entity types #850
Conversation
mickmis
left a comment
mickmis
left a comment
There was a problem hiding this comment.
LGTM, there are just two things I'm not sure were intended for that PR? See comments.
| def _add_section_numbers(elements: Sequence[marko.element.Element]) -> None: | ||
| heading_level = 2 | ||
| levels = [0] | ||
| headings = [None] |
There was a problem hiding this comment.
Are the changes in that file expected? They are not covered by the PR description.
But they LGTM in themselves.
There was a problem hiding this comment.
I did not mean to include these changes in this PR, but given you've reviewed them now and LGTM, I'll update the description to mention them and then keep them in the PR.
| ### [Ensure clean workspace test step](clean_workspace.md) | ||
| ### Ensure clean workspace test step | ||
|
|
||
| <!-- TODO(Shastick): Why do we need to reclean the workspace at this point? We already ensured it was clean before starting the test; don't we know exactly what happened in the test and therefore know that it's already clean (or something failed)? If a previous test case created something that we don't need/want in later test cases, the original test case should clean up at the end of the test case. --> |
There was a problem hiding this comment.
Are the addition of those TODOs intended?
There was a problem hiding this comment.
Yes -- it seemed difficult to create separate Issues that referred to exactly the right place and it may certainly be the case that these TODOs are not actually problems. My in-between solution was to add these with the idea that Julien can search for TODO(Shastick) when he returns and address them then.
2 participants
Previously, the clean_workspace.md test step fragment was a catch-all that documented cleaning out subscriptions, operational intent references, and constraint references regardless of whether all those different entity types were cleaned out or needed to be cleaned out. This PR splits clean_workspace.md into clean_workspace_subs.md, clean_workspace_op_intents.md, and clean_workspace_constraints.md and narrows each test scenario leveraging that fragment to a tighter scope describing what is actually happening.
In a few test scenarios, the code to clear subscriptions is removed because there is no need to have subscriptions removed in order to accomplish the test.
A few hidden TODOs are added to address later. Mislabeled constraint variables and comments in a scenario are fixed.
A small amount of improved error description code for the globally expanded report is included.