Skip to content

regression: add tests for the supplicant plugin framework - #482

Merged
jforissier merged 1 commit into
OP-TEE:masterfrom
anisyanka:plugin-framework-test
Feb 4, 2021
Merged

regression: add tests for the supplicant plugin framework#482
jforissier merged 1 commit into
OP-TEE:masterfrom
anisyanka:plugin-framework-test

Conversation

@anisyanka

Copy link
Copy Markdown
Contributor

Add test cases for checking functionalitu of supplicant plugins:

  • Ability to pass values and arrays to/from a plugin
  • Right handle situations with bad input parameters
  • Right handle situations with calling an unknown plugin

Signed-off-by: Aleksandr Anisimov a.anisimov@omprussia.ru

@anisyanka

Copy link
Copy Markdown
Contributor Author

Comment thread host/supp_plugin/test_supp_plugin.c Outdated
}

struct plugin_method plugin_method = {
"test",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please name the fields you're initializing. The NULL below can be skippen then also.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@anisyanka
anisyanka force-pushed the plugin-framework-test branch from 9c640ad to 630c8e2 Compare January 25, 2021 12:18
@anisyanka

Copy link
Copy Markdown
Contributor Author

All comments have been addressed

Comment thread host/xtest/regression_1000.c Outdated
TEEC_NONE);

ADBG_EXPECT_TEEC_SUCCESS(c, TEEC_InvokeCommand(&session,
TA_SUPP_PLUGIN_PASS_VALUES, &op,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're aligning with wrong (
This comment applies to a few places below too.

#define TA_SUPP_PLUGIN_TEST_UUID { 0x380231ac, 0xfb99, 0x47ad, \
{ 0xa6, 0x89, 0x9e, 0x01, 0x7e, 0xb6, 0xe7, 0x8a } }

#define TA_SUPP_PLUGIN_PING 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename these TA commands as TA_SUPP_PLUGIN_CMD_*

#ifndef TA_SUPP_PLUGIN_TEST_H
#define TA_SUPP_PLUGIN_TEST_H

#define TA_SUPP_PLUGIN_TEST_UUID { 0x380231ac, 0xfb99, 0x47ad, \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename to TA_SUPP_PLUGIN_UUID, the TEST part is redundant.
Please drop the _test part from the name of this file too.

Comment thread host/xtest/regression_1000.c Outdated

/*
* TA will ping the test plugin during open session operation.
* Supplicant plugins are optional: warn and nicely exit if not found

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional? I can't see that in OP-TEE/optee_client#239

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks.

Comment thread ta/supp_plugin/ta_entry.c
return tee_invoke_supp_plugin(&nulluuid, 0, 0, data, inlen, NULL);
}

TEE_Result TA_InvokeCommandEntryPoint(void *pSessionContext __unused,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these TA functions could be replaced by a single TA function (the os_test TA would be a good match then) with a flexible interface allowing to put most of the logic in xtest. If you'd rather keep it as is that's fine too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to keep it as it is, thanks

@anisyanka

Copy link
Copy Markdown
Contributor Author

All comments have been addressed

@jenswikl

jenswikl commented Feb 4, 2021

Copy link
Copy Markdown
Contributor

Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

Add test cases for checking functionality of supplicant plugins:
 - Ability to pass values and arrays to/from a plugin
 - Right handle situations with bad input parameters
 - Right handle situations with calling an unknown plugin

Signed-off-by: Aleksandr Anisimov <a.anisimov@omprussia.ru>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>
@anisyanka
anisyanka force-pushed the plugin-framework-test branch from 80204b0 to f77a9de Compare February 4, 2021 08:09
@anisyanka

anisyanka commented Feb 4, 2021

Copy link
Copy Markdown
Contributor Author

Squashed the fixup commits and updated the commit message.
Thank you for the review!

@jforissier

Copy link
Copy Markdown
Contributor

IBART is passing, all good! Thanks @anisyanka.

@jforissier
jforissier merged commit 01f6f06 into OP-TEE:master Feb 4, 2021
@anisyanka
anisyanka deleted the plugin-framework-test branch February 4, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants