Skip to content

Conversation

@sivakondri-CT
Copy link
Contributor

@sivakondri-CT sivakondri-CT commented Dec 18, 2025

This file contains helper functions related to IoT tests, which can be imported and reused wherever IoT test execution or reporting is required.

Copy link
Collaborator

@memnochproxy memnochproxy left a comment

Choose a reason for hiding this comment

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

These functions all need better documentation about what parameters are expected. If someone passes in a dictionary or an object, please document what the expected keys are and what the behavior is when the keys are missing.

Please do not nest functions: move copy_into_report() function out of the parent function and document it and the return value indicating success. Also, please put some some basic check on what the absolute path is. Collections of reports can grow to many millions of directories, and also the value of '.' (current directory) could also evaluate to '/' or 'c:' which should not be a legitimate input. Please consider a reasonable limit to the number of directories that method should scan and throw an exception when the method reaches that limit.
Also

@sivakondri-CT
Copy link
Contributor Author

These functions all need better documentation about what parameters are expected. If someone passes in a dictionary or an object, please document what the expected keys are and what the behavior is when the keys are missing.

Please do not nest functions: move copy_into_report() function out of the parent function and document it and the return value indicating success. Also, please put some some basic check on what the absolute path is. Collections of reports can grow to many millions of directories, and also the value of '.' (current directory) could also evaluate to '/' or 'c:' which should not be a legitimate input. Please consider a reasonable limit to the number of directories that method should scan and throw an exception when the method reaches that limit. Also

thanks for the suggestions. I’ll improve the IoT helper by documenting expected parameters, required keys, and fallback behavior for missing values. I’ll also move copy_into_report() into a standalone helper with a documented return value, and add safety checks to reject unsafe paths (/, ., c:) along with a bounded directory traversal limit that raises an exception when exceeded.

@sivakondri-CT sivakondri-CT force-pushed the iot-helper branch 2 times, most recently from 0a90dbf to d2d73e2 Compare December 22, 2025 06:36
…ting

Signed-off-by: sivakondri-CT <kondru.sankar@candelatech.com>
@sivakondri-CT
Copy link
Contributor Author

These functions all need better documentation about what parameters are expected. If someone passes in a dictionary or an object, please document what the expected keys are and what the behavior is when the keys are missing.
Please do not nest functions: move copy_into_report() function out of the parent function and document it and the return value indicating success. Also, please put some some basic check on what the absolute path is. Collections of reports can grow to many millions of directories, and also the value of '.' (current directory) could also evaluate to '/' or 'c:' which should not be a legitimate input. Please consider a reasonable limit to the number of directories that method should scan and throw an exception when the method reaches that limit. Also

thanks for the suggestions. I’ll improve the IoT helper by documenting expected parameters, required keys, and fallback behavior for missing values. I’ll also move copy_into_report() into a standalone helper with a documented return value, and add safety checks to reject unsafe paths (/, ., c:) along with a bounded directory traversal limit that raises an exception when exceeded.

Hi @a-gavin , @memnochproxy I’ve updated the PR addressing the mentioned suggestions. Please review when you have some time. Thanks!

Copy link
Collaborator

@smileyrekiere smileyrekiere left a comment

Choose a reason for hiding this comment

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

The changes requested by Jed appear to have been met.

@memnochproxy memnochproxy merged commit 77dc22d into greearb:master Dec 29, 2025
2 checks passed
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