Conversation
superstian
force-pushed
the
add_usb_device_integation_test
branch
3 times, most recently
from
cbfab50 to
9ca036f
Compare
| // Should this fail? | ||
| using var _libUsb2 = new LibUsb(_loggerFactory); |
There was a problem hiding this comment.
I would say no maybe, but calling _libUsb2.Initialize() on the second instance should. It's a .NET convention that constructors don't throw. Normally constructors should not throw, but in this case I'm not entirely sure. Maybe it's better if we treat the LibUsb type as a singleton?
| } | ||
| } | ||
|
|
||
| using var _libUsb = new LibUsb(_loggerFactory); |
There was a problem hiding this comment.
I committed some changes to TestDeviceSource.cs, see 2c50c2c
Maybe we could use the TestDeviceSource and do something like this to skip the test if device not found:
_deviceSource.OpenUsbDeviceOrSkip().Dispose();?
There was a problem hiding this comment.
Or we could implement a using var token = _deviceSource.ClaimDeviceOrSkip() that would find a suitable USB device and reserve for the test, without opening it?
There was a problem hiding this comment.
I'm not a big fan of skipping tests like this. It can lead to tests getting skipped in CI without us knowing it. We have seen this in the past with other similar projects
It would be nice to have additional logic with some sort of configuration where we can define what devices to test and what tests to run on a specific system.
There was a problem hiding this comment.
I'm not a big fan of skipping tests like this. It can lead to tests getting skipped in CI without us knowing it. We have seen this in the past with other similar projects
It would be nice to have additional logic with some sort of configuration where we can define what devices to test and what tests to run on a specific system.
The point of the skip tests design is:
- convenience for integrators/contributors that don't have access to all supported devices
- easily prototype and add tests for new devices in a dev environment, while still running as many tests as possible
- make it apparent what the problem is: skip=DUT not available, fail=Something is wrong
I don't see the downside: In a CI environment with access to a devices with e.g. vendor class interface, we can easily run all tests for category "UsbVendorClassDevice" and fail on skip. Or if we had a CI environment will all supported devices on one host, we could run all tests and fail on skip.
There was a problem hiding this comment.
This will help a bit in enforcing how test skip is used in this project: deb247c
There was a problem hiding this comment.
Should this file be named WindowsHandles or similar. I guess we have no plan of making this work on any other OS?
There was a problem hiding this comment.
I think we should make another test project for tests that are Windows platform specific. IMO this makes it easier to organize platform specific code/dependencies and choose what projects to run on what node in CI.
We can make the TestInfrastructure types public and share them between all the test projects. The cleanest is likely to extract the shared TestInfrastructure to a separate project.
There was a problem hiding this comment.
Sounds like a good plan.
SetRequiredProductId and SetRequiredSerialNumber
tmittet
force-pushed
the
add_usb_device_integation_test
branch
from
2c50c2c to
ab16816
Compare
|
Rebased on main and extracted Windows specific tests to separate project |
when on a Windows runner
tmittet
force-pushed
the
add_usb_device_integation_test
branch
2 times, most recently
from
df8c248 to
f184711
Compare
tmittet
force-pushed
the
main
branch
3 times, most recently
from
2b3dd7b to
f967b1c
Compare
2 participants
Adds a new test class for serialized integration testing. Currently WIP