search

Nitrokey / pynitrokey

Python client for Nitrokey devices
Apache License 2.0
93 stars 28 forks source link

Add SE050 test #423

Closed sosthene-nitrokey closed 8 months ago

sosthene-nitrokey commented 11 months ago

This PR adds a test that is introduced in the admin app with https://github.com/Nitrokey/admin-app/pull/5

Note that it takes around 30seconds to complete, it's not crashing.

sosthene-nitrokey commented 10 months ago

Why the control of the tests is placed in the firmware instead of the test calling code?

What do you mean by "the control of the test"?

I think pytest would be better to use here, but its done already.

The goal here is to get info from beta testers that the SE050 driver works as expected with NK3 that are already deployed. If doing so through pytest we likely wouldn't get feedback since users are unlikely to use it.

szszszsz commented 10 months ago

What do you mean by "the control of the test"?

You are essentially calling a fixed test routine in the firmware, and waiting for the results. Instead you could construct test cases on the calling side, and use a proper direct communication channel to se050.

If doing so through pytest we likely wouldn't get feedback since users are unlikely to use it.

That explains things. I do not think though running pytest itself is that much trouble for a beta tester, but that's my opinion.

sosthene-nitrokey commented 10 months ago

You are essentially calling a fixed test routine in the firmware, and waiting for the results. Instead you could construct test cases on the calling side, and use a proper direct communication channel to se050.

I wanted to test the actual APDU generation from the driver to test it, so all commands to the SE050 needs to be run from rust code. This is meant to test the driver in real conditions. Communicating to the SE050 directly from nitropy would make that much less useful, even if it were to test the T=1 over I2C part of the driver.

I do not think though running pytest itself is that much trouble for a beta tester, but that's my opinion.

I agree, but running pytest is much less discoverable while nk3 test is documented and used.

daringer commented 9 months ago

this does not interfere with anything else, should we merge this ?

robin-nitrokey commented 9 months ago

Running this with the se050-test-app feature disabled now gives me:

[4/5]   se050       SE050                       FAILURE     object of type 'type' has no len()
sosthene-nitrokey commented 9 months ago

Running this with the se050-test-app feature disabled now gives me:

[4/5] se050       SE050                       FAILURE     object of type 'type' has no len()

Can you try again? This should be fixed I believe.

robin-nitrokey commented 9 months ago

Yes, fixed.