-
Notifications
You must be signed in to change notification settings - Fork 55
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
Test configuration management on main device #1875
Test configuration management on main device #1875
Conversation
Beware that the refactoring of configuration management has been merged. Hence, if we want to compare before/after behavior we will have to compare with the |
Robot Results
Passed Tests
|
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.
The test looks fine. A few more tests cases that could be added for improved coverage (not necessarily to be added in this PR itself):
- Updates to the
c8y-configuration-plugin.toml
are automatically picked up and the supported config list is updated - Set non-existent config file with the desired file permissions
- Updating
c8y-configuration-plugin.toml
from the cloud updates the supported config list automatically
I can think of some more negative test cases like setting broken/invalid c8y-config-plugin.toml
, setting non-existent config-type etc. But, not sure if they're all relevant here at a system test level.
tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot
Outdated
Show resolved
Hide resolved
I just re-read the list of extra test cases and I think it would be definitely worthwhile extending this PR to cover them, thanks for the suggestions. |
2f35c97
to
7e6d079
Compare
For "sed", I can see the notify stream has events for the tmp file creation as well as renaming the tmp file to the c8y-configuration-plugin.toml but the notify stream match (crates/common/tedge_utils/src/notify.rs:73) drops the modification events which renames the tmp file to c8y-configuration-plugin.toml because we only match on modification events which have kind "data" but they're multiple event types created here which are Modify(Name(From)), Modify(Name(To)) and Modify(Name(Both)) the types can be seen at https://docs.rs/notify/latest/notify/event/enum.ModifyKind.html and these events are dropped so I'm not surprised this fails. @reubenmiller On the other hand when manually testing the "mv" I can see appropriate logs all the way from the notify stream to the ConfigManagerMessageBox sending a message with an updated config but the Robotest fails and I'm not why. |
7e6d079
to
6e8f5a2
Compare
de068eb
to
91efb40
Compare
@jmshark There is actually a different here. I posted a comment describing when a move will work and when it won't. #1896 (comment) |
1707e30
to
774f007
Compare
tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot
Outdated
Show resolved
Hide resolved
tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot
Outdated
Show resolved
Hide resolved
tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot
Outdated
Show resolved
Hide resolved
Signed-off-by: Reuben Miller <reuben.d.miller@gmail.com>
a9eca5e
to
e1769ec
Compare
Proposed changes
Add Cumulocity configuration management integration tests (for the main device)
Add test cases to cover the following:
c8y-configuration-plugin.toml
are automatically picked up and the supported config list is updated. Though some test cases are skipped due to Some file modification methods do not trigger the file-changed handler in c8y-configuration-plugin #1896c8y-configuration-plugin.toml
from the cloud updates the supported config list automatically. Test fails due to regressionc8y-configuration-plugin
supported configuration types is not updated via cloud update #1883Types of changes
Paste Link to the issue
Checklist
cargo fmt
as mentioned in CODING_GUIDELINEScargo clippy
as mentioned in CODING_GUIDELINESFurther comments