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

Create tasks that will follow the Documentation instructions #2878

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

Conversation

gligorisaev
Copy link
Contributor

Proposed changes

For documentation checking purposes I am creating collection of End to End tasks which will follow the descriptions/instructions from thinedge documentation.
These tasks are running on an Raspberry PI.
The idea behind is that as soon as some task fail that would mean that changes in the documentations are needed or some regression mailfunction has been detected.

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


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 May 15, 2024 08:25
@gligorisaev gligorisaev marked this pull request as draft May 15, 2024 08:26
Copy link
Contributor

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
433 0 3 433 100 58m56.358857s

@gligorisaev gligorisaev marked this pull request as ready for review July 22, 2024 09:19
Comment on lines 88 to 89
Should Contain ${OUTPUT} tedge - CLI tool use to control and configure thin-edge.io
Should Contain ${OUTPUT} tedge - CLI tool use to control and configure thin-edge.io
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicated line.

Suggested change
Should Contain ${OUTPUT} tedge - CLI tool use to control and configure thin-edge.io
Should Contain ${OUTPUT} tedge - CLI tool use to control and configure thin-edge.io
Should Contain ${OUTPUT} tedge - CLI tool use to control and configure thin-edge.io

Comment on lines 182 to 191
# Custom Setup
# [Documentation] Initializes the device environment.
# ... Sets up the device, transfers necessary packages,
# ... installs them, and configures Cumulocity for connectivity.
# ${DEVICE_SN}= Setup skip_bootstrap=True
# Set Suite Variable ${DEVICE_SN}
# ${log} Transfer To Device target/aarch64-unknown-linux-musl/packages/*.deb /home/pi/
# Execute Command sudo dpkg -i *.deb
# Log Installed new packages on device

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be deleted

Comment on lines 137 to 139
Verify Measurement In Cumulocity
Device Should Have Measurements type=environment minimum=1 maximum=1

Copy link
Contributor

Choose a reason for hiding this comment

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

It would make more sense to make this a Keyword (function) that takes an argument for the type, then you can reuse it for the other measurements.

Verify Measurement In Cumulocity
    [Arguments]        ${type}
    Device Should Have Measurements    type=${type}    minimum=1    maximum=1

Then the usage is just:

Verify Measurement In Cumulocity    type=environment


Optional: Linux distributions without systemd curl
${OUTPUT} Execute Command curl -fsSL https://thin-edge.io/install-services.sh | sh -s ignore_exit_code=True
#Not verifiing this test step because the test running in container already exists: tests/RobotFramework/tests/installation/install_on_linux.robot
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#Not verifiing this test step because the test running in container already exists: tests/RobotFramework/tests/installation/install_on_linux.robot
#Skip verification step as system tests already exist: tests/RobotFramework/tests/installation/install_on_linux.robot


Optional: Linux distributions without systemd wget
${OUTPUT} Execute Command wget -O - https://thin-edge.io/install-services.sh | sh -s
#Not verifiing this test step because the test running in container already exists: tests/RobotFramework/tests/installation/install_on_linux.robot
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#Not verifiing this test step because the test running in container already exists: tests/RobotFramework/tests/installation/install_on_linux.robot
#Skip verification step as system tests already exist: tests/RobotFramework/tests/installation/install_on_linux.robot

Comment on lines 141 to 144
Verify Combined Measurement In Cumulocity
[Documentation] Verify the combined measurement in Cumulocity.
# Add implementation for checking the combined measurement in Cumulocity
Log Verified combined measurement in Cumulocity
Copy link
Contributor

Choose a reason for hiding this comment

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

Now you can use the same Verify Measurement In Cumulocity keyword to verify that this message is also published.


Transfer Aarch64 Packages
[Documentation] Transfers Aarch64 architecture packages to the device.
${log} Transfer To Device target/aarch64-unknown-linux-musl/packages/*.deb /home/pi/
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't be using /home/pi path...use some other path like:

Suggested change
${log} Transfer To Device target/aarch64-unknown-linux-musl/packages/*.deb /home/pi/
${log} Transfer To Device target/aarch64-unknown-linux-musl/packages/*.deb /var/local/share/


Transfer Armv7l Packages
[Documentation] Transfers ARMv7l architecture packages to the device.
${log} Transfer To Device target/armv7-unknown-linux-musleabihf/packages/*.deb /home/pi/
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't be using /home/pi path...use some other path like:

Suggested change
${log} Transfer To Device target/armv7-unknown-linux-musleabihf/packages/*.deb /home/pi/
${log} Transfer To Device target/armv7-unknown-linux-musleabihf/packages/*.deb /var/local/share/

Custom Teardown
[Documentation] Cleans up the device environment.
... Uninstalls ThinEdgeIO, removes packages and scripts, and retrieves logs.
Transfer To Device ${CURDIR}/uninstall-thin-edge_io.sh /home/pi/uninstall-thin-edge_io.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Transfer To Device ${CURDIR}/uninstall-thin-edge_io.sh /home/pi/uninstall-thin-edge_io.sh
Transfer To Device ${CURDIR}/uninstall-thin-edge_io.sh /var/local/share/uninstall-thin-edge_io.sh

Copy link
Contributor

Choose a reason for hiding this comment

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

Though there are other places which also use the /home/pi folder...best to avoid this all together.

... installs them, and configures Cumulocity for connectivity.
${DEVICE_SN}= Setup skip_bootstrap=True
Set Suite Variable ${DEVICE_SN}
${log} Transfer To Device target/aarch64-unknown-linux-musl/packages/*.deb /home/pi/
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes you are running on a specific architecture, e.g. aarch64, so you'll need to the same arch detecting logic.

for path in $installation_paths; do
if [ -d "$path" ]; then
sudo rm -rf "$path/tedge"
sudo rm -rf "$path/tedge_*"
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work as expected, as the shell wildcard is within the double quotes, meaning it will not be expanded to folders with tedge_* in the name.

Suggested change
sudo rm -rf "$path/tedge_*"
sudo rm -rf "$path/tedge_"*

@reubenmiller
Copy link
Contributor

@gligorisaev What is the state of this PR, it seems to be still a work-in-progress. In that case it would be better to set it to the Draft state. It is unclear which comments have been addressed or not as there aren't any links to the commits which address the comments.

@gligorisaev gligorisaev marked this pull request as draft October 21, 2024 12:08
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.

4 participants