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.

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 system test for the SmartREST templates #3202

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gligorisaev
Copy link
Contributor

@gligorisaev gligorisaev commented Oct 24, 2024

To mitigate this risk that any future changes to the codebase could introduce errors or regressions that go unnoticed, which might still pass the build workflow due to the lack of automated coverage for this feature, I have implemented a system test suite for SmartREST templates based on our documentation.
The purpose of this test suite is to ensure that any updates or modifications related to SmartREST templates are properly verified during the build process.

Test Suite Details:

The test suite covers:

1.	Verifying the non-existence of SmartREST templates before starting.
2.	Adding new SmartREST templates and verifying their presence in the system.
3.	Checking the coexistence of multiple templates.
4.	Overwriting templates using the set command.
5.	Removing individual templates and verifying the removal.
6.	Clearing all SmartREST templates using the unset command.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3199

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@gligorisaev gligorisaev requested a review from a team as a code owner October 24, 2024 11:23
@gligorisaev gligorisaev marked this pull request as draft October 24, 2024 11:23
Copy link
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

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

That's a good start. However the most important (and tricky) part is the invocation from Cumulocity https://thin-edge.github.io/thin-edge.io/operate/c8y/smartrest-templates/#example-creating-a-custom-operation

Note: the links given in the PR's description are broken.


Verify Non-Existing SmartREST Templates
${output}= Execute Command tedge config get c8y.smartrest.templates
Should Match Regexp ${output} ^\s*(\\[\\]|\bnone\b|null)\s*$
Copy link
Contributor

Choose a reason for hiding this comment

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

Tedge config will never returns none or null.

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 thoughts behind this were instead of assuming the output will be [], to ensure the keyword can handle different potential outputs for no templates, making the test more robust, not for now but also in the future. I can change it to the state that I had before Should Contain ${output} []

Copy link
Contributor

Choose a reason for hiding this comment

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

c8y.smartrest.templates is a list of templates, hence none and null values are to be rejected as errors.

@rina23q
Copy link
Member

rina23q commented Nov 4, 2024

However the most important (and tricky) part is the invocation from Cumulocity https://thin-edge.github.io/thin-edge.io/operate/c8y/smartrest-templates/#example-creating-a-custom-operation

Exactly, this is the missing part that we found out during the work on #3189. All of our existing custom operations tests use the topic c8y/s/ds, which is for static templates.

For example, (c8y_RelayArray)

[exec]
topic = "c8y/s/ds"
on_message = "519"
result_format = "csv"
command = "/etc/tedge/operations/set_relay.sh"
timeout = 10

However, as you see in our doc, we also supports the topics for the custom templates.
For example, (copied from the doc), c8y/s/dc/custom_devmgmt.

[exec]
command = "/usr/bin/set_wifi"
topic = "c8y/s/dc/custom_devmgmt"
on_message = "dm101"

During the work of #3189, accidentally it removed the support for the custom template topic. However, all tests passed and we couldn't find it easily because we don't have a test case to cover it. That's the motivation to request you to create a test following the guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants