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

tests/sys/shell_lock: increase robustness of the test #20962

Merged

Conversation

maribu
Copy link
Member

@maribu maribu commented Nov 7, 2024

Contribution description

Before the test used the help command to verify that access to the shell is present. While this does have the benefit of not requiring a custom command, it does have some robustness issues:

  • When new default commands get added, this test will fail
  • When the help message of a command gets reworded, the test will fail
  • When the order of commands in the test will change, the test will fail
    • Note that with the XFA based SHELL_COMMAND() macro, we do not guarantee any particular order. Specifically, the order can change with implementation details.

Testing procedure

The test should pass.

Issues/PRs references

#20958 has shown that changing implementation details of SHELL_COMMAND() does affect the order of commands in the help list.

Before the test used the `help` command to verify that access to the
shell is present. While this does have the benefit of not requiring a
custom command, it does have some robustness issues:

- When new default commands get added, this test will fail
- When the help message of a command gets reworded, the test will fail
- When the order of commands in the test will change, the test will fail
    - Note that with the XFA based `SHELL_COMMAND()` macro, we do not
      guarantee any particular order. Specifically, the order can change
      with implementation details.
@maribu maribu added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 7, 2024
@maribu maribu requested a review from HendrikVE November 7, 2024 09:52
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Nov 7, 2024
@riot-ci
Copy link

riot-ci commented Nov 7, 2024

Murdock results

✔️ PASSED

abb666b tests/sys/shell_lock: increase robustness of the test

Success Failures Total Runtime
20 0 22 01m:58s

Artifacts

@mguetschow mguetschow added this pull request to the merge queue Nov 7, 2024
Merged via the queue into RIOT-OS:master with commit 76b9b71 Nov 7, 2024
29 checks passed
@maribu maribu deleted the tests/sys/shell_lock/test-robustness branch November 7, 2024 12:34
@maribu
Copy link
Member Author

maribu commented Nov 7, 2024

Thx a bunch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants