-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
Add general guidelines for writing test scripts regarding RFC 2119 Ke… #154
Add general guidelines for writing test scripts regarding RFC 2119 Ke… #154
Conversation
…ywords (standards/issues/#447) Signed-off-by: Patrick Thiem <[email protected]>
@cah-patrickthiem is the text ready for review from your point of view or are you still working on it? If you're finished feel free to request a review from me :-) I already have a few remarks/suggestions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a real review, just a remark and a request: once these guidelines are merged, please add a link to https://github.com/SovereignCloudStack/standards/blob/main/.github/ISSUE_TEMPLATE/stabilize-standard.md?plain=1#L15
You are completely right here. The branch is out of sync. Also, the subfolders were just a quick catch of me, I understand that this needs to be discussed, but right now I also think this is not necessary. |
…2119 Keywords (standards/issues/#447) Signed-off-by: cah-patrickthiem <[email protected]>
Pushed a !fixup to address several inconsistensies. @martinmo the text itself is ready for review, feel free to check it. Also, if you want to add more content regarding tests, feel free to do so. |
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
After the remarks I just made, I have a suspicion that these docs may actually become a standard one day. But it's fine to start out with docs. |
…ded examples for RFC2119 keyword use for standard documents and tests.
… and added examples for RFC2119 keyword use for standard documents and tests. Signed-off-by: cah-patrickthiem <[email protected]>
…#447 Signed-off-by: cah-patrickthiem <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed it and left some suggestions for some minor issues I have found. Overall this looks really promising and we should get this merged soon 👍
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
One more thing, because I just read @mbuechse's comment #154 (comment) again. We should definitely discuss whether to specify in the document that all channels output should go to I'm in favor of putting it into this document. |
Added this as well. |
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
|
||
In addition, a test **MUST** exit with a non-zero exit code (e.g., via `sys.exit(…)`) if there are any ERROR or CRITICAL messages, thus signaling a failure to meet a standard. | ||
|
||
It is crucial to determine and explicitly state within this document the expected conventions for logging output, specifically regarding the redirection of all channel outputs to standard error (**stderr**), which aligns with Python's default logging behavior. The format which **MUST** be used is **CHANNEL(MESSAGE)**, where **CHANNEL** represents the log level (e.g., DEBUG, INFO, WARNING, ERROR, CRITICAL) and **MESSAGE** encapsulates the actual log message. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is crucial to determine and explicitly state within this document the expected conventions for logging output
This sounds like a prompt for the reader to determine the conventions and should be shortened to something like "the scripts are expected to ...".
Instead of CHANNEL(MESSAGE)
the actual format we use is CHANNEL: MESSAGE
. Example:
INFO: Checking cluster specified by context 'my-cluster' in /path/to/.kube/config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
With regard to my comment:
I double checked with the existing scripts in the standards repo. Some scripts explicitly set the logging stream to Maybe we should leave the |
I wouldn't say strong, but I think the bit with stdout is a mistake, and stderr should be a recommendation at the very least. |
Fyi I talked to @martinmo and we came to the conclusion to make "stderr" a MUST for test scripts. |
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
contributor-docs/operations/tests/rfc2119-keyword-test-guide.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
SovereignCloudStack#154) * Add general guidelines for writing test scripts regarding RFC 2119 Keywords (standards/issues/#447) --------- Signed-off-by: cah-patrickthiem <[email protected]>
This PR adds a document for general guidelines in regard of writing test scripts, especially on how to use RFC2119 keywords.
Resolves: SovereignCloudStack/standards#447