Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Drivelist tests #962

Open
wants to merge 2 commits into
base: development
Choose a base branch
from
Open

Conversation

lbrooks
Copy link
Contributor

@lbrooks lbrooks commented Dec 3, 2024

Issue: devicelist.list() could return an incorrect mountpoint path if on the first call the appropriate descriptor was found without any mountpoints and the devicelist.list() function returned values in a different order.

Changes:

  • Root Fix: Replaced the nested while loop with fail fast continue statements
  • Changed the ordering of the mountpoint check to prevent a failure when accessing a value in an array when the array is undefined
  • Switched from two string contains to a single RegExp test
  • Fixed the declared types
  • Added full test coverage

Possible Issue Uncovered:

  • Method will loop infinitely until a device with the appropriate descriptor and a valid mountpoint is found.

I have added logic to support enforcing a maximum number of retries and tests to ensure the max is respected but this PR does not put a maximum retry count on the method.

If the devicelist function returned objects in a different order
the existing function could return an incorrect mountpoint path.

Fixed this by refactoring the method.

Additionally, it looks like this method will loop forever until
a valid device and mountpoint is found. Unsure if limiting this
to a maximum retry count would break things, so leaving as
unlimited retries.

Signed-off-by: Lawrence Brooks <84.lbrooks@gmail.com>
Signed-off-by: Lawrence Brooks <84.lbrooks@gmail.com>
@lbrooks lbrooks marked this pull request as ready for review December 3, 2024 04:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant