Add a NoDetails variant for MockError

[cdunster]

Currently, in no_std projects it is required to include the std library to test with mocks that return errors, this is because the only supported MockError is one that takes a std::io::ErrorKind. By adding this new variant that doesn't depend on the std library we can keep the project no_std even for testing.

I am open to other names if NoDetails is not a good fit. It might be nice to have separate errors for each type of mock e.g. MockError::Adc(AdcError::InvalidData) but the way in this PR requires fewer changes.

Thanks.

[newAM]

I'm not sure I understand the purpose of this change. Can you give an example of where this is useful?

[dbrgn]

Currently, in no_std projects it is required to include the std library to test with mocks that return errors

Is that a problem? Currently embedded-hal-mock does require std, but I don't see any downsides to having to enable std for tests.

[cdunster]

I'm not sure I understand the purpose of this change. Can you give an example of where this is useful?

@newAM, it just removes the current dependency on the standard library when using errors in mocks. This dependency means it is not possible to run some tests on target (devices that can't use the standard library).

Is that a problem? Currently embedded-hal-mock does require std, but I don't see any downsides to having to enable std for tests.

@dbrgn, the main problem is that it means tests cannot be run on target. Other than that it adds a bit of extra boilerplate as then we need to use #![cfg_attr(not(test), no_std)] in every project and when running on target we need to disable all tests that use errors with #[cfg(not(target_arch = "arm"))] and change the project to #![cfg_attr(all(not(test), not(target_arch = "arm")), no_std)] all just to use errors from the standard library. Also, I believe that io errors are supposed to be used for file operations so I think custom errors could be more descriptive.

[dbrgn]

Well, it's not just the io errors. For example, we use Vec in a few places in the source (see https://github.com/search?q=repo%3Adbrgn%2Fembedded-hal-mock%20vec&type=code), and probably some other std-only data structures as well.

While I'm aware that there's a bit of boilerplate necessary for projects, the alternative would be increasing complexity of this project for all developers and contributors, by not using std and not being allowed to use the heap. So right now I'm inclined to stick to std, unless there are compelling arguments that outweigh the maintenance complexity 🙂

[newAM]

@newAM, it just removes the current dependency on the standard library when using errors in mocks. This dependency means it is not possible to run some tests on target (devices that can't use the standard library).

I am sorry to keep asking, but can you further clarify this? Code would help, if you can share it.

I am confused because:

• My understanding is that your goal is to compile this crate for a no_std target.
• After this change the crate will still require std.

To make the crate compile for embedded targets requires adding #![no_std] and changing all the use std:: declarations to use core::, which this pull-request doesn't cover.

[cdunster]

Hi @dbrgn and @newAM, sorry for the confusion with this, I think that I've not made it very clear when I'm talking about embedded-hal-mock vs talking about my project that has a library and a binary that for ease I'll call embedded-lib and embedded-bin.

I'd first like to state that I appreciate this crate very much and the work done here so I hope the correct tone comes across in this message.

I think that my original point is hard to explain, but I will try. However, I'd like to ask: why does this crate use io::Error's instead of custom errors? I know it mostly doesn't matter but it seems strange to me that a GPIO pin would return an IO file not found error or something instead of something more specific to GPIO pins.

Right now I have:

• embedded-lib: a no_std library that contains most of the business logic and can be built for both cortex-m (thumb) targets and x86_64 targets.
• embedded-bin: designed to only be built for thumb targets and thus should not use the standard library.

Most of the tests are in embedded-lib and do indeed run on x86_64 right now so can use the standard library, these tests depend on embedded-hal-mock as a dev-dependency but that is okay because the tests only run on x86_64 so can use non-no_std crates as dependencies. However, just because embedded-lib depends on crates that use the standard library does not mean that embedded-lib itself needs to depend on the standard library so I can leave embedded-lib as #[no_std] and it will be built without the standard library for tests and production but then for tests, embedded-hal-mock requires the standard library so it is added as a dependency automatically.
However, if I want to test the error flow then I need to use .with_error(MockError::Io(ErrorKind::NotConnected)) and as io::ErrorKind is part of the standard library and it is the only error type that is allowed then I need to construct an io::ErrorKind type in the unit test within embedded-lib at this point embedded-lib now directly depends on the standard library instead of indirectly through embedded-hal-mock therefore I need to change embedded-lib to be built without the standard library for production (#[no_std]) but with the standard library for tests so I need to add something like #![cfg_attr(not(test), no_std)] to embedded-lib to use the standard library directly and allow construction of io::ErrorKind types meaning that embedded-lib now has a direct dependency on the standard library for tests just to test a few error flows.

I know it doesn't matter because the result is the same; directly or indirectly through embedded-hal-mock, embedded-lib depends on the standard library. It's more of a feeling/opinion that dependences should only be added where needed. If I decide later to remove these tests or not use embedded-hal-mock or whatever then I will still depend on the standard library and I need to remember to remove the dependency, so this leads me to the question of "why does this crate use io::Error's instead of custom errors?"

I hope I explained it a bit better this time, thanks.

P.S. I would be interested in looking at making this crate no_std if you would like. For things like Vec then this crate could just depend on alloc instead of std.

[dbrgn]

I think the MockError::Io should be easy to replace with a custom error variant.

Right now I don't know what the implications of making this library no_std are. If you want, you could give it a try! However, given that there is currently a solution for this (using #[cfg(test)] everywhere), the decision whether to merge such a change or not, would depend on how much complexity is added and what the implication to consumers of the library would be.