Skip to content

xtest: add test entry for transfer list#788

Merged
jforissier merged 1 commit into
OP-TEE:masterfrom
raymo200915:fw_handoff_ut
May 6, 2025
Merged

xtest: add test entry for transfer list#788
jforissier merged 1 commit into
OP-TEE:masterfrom
raymo200915:fw_handoff_ut

Conversation

@raymo200915
Copy link
Copy Markdown
Contributor

Add test entry for transfer list in xtest 1001.

@raymo200915
Copy link
Copy Markdown
Contributor Author

Add macros and helper function to avoid returning error for not supported tests.

Comment thread host/xtest/regression_1000.c Outdated
Do_ADBG_BeginSubCase(c, "Core transfer list self tests");

(void)ADBG_EXPECT_TEEC_SUCCESS(c, TEEC_InvokeCommand(
(void)ADBG_EXPECT_TEEC_SUCCESS_OR_NOTSUPPORT(c, TEEC_InvokeCommand(
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 use a helper variable to store the result from TEEC_InvokeCommand() to improve the line breaks, and drop the (void).
I know the rest of this function is different, but we need to start somewhere to improve the pattern.

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.

Comment thread host/xtest/xtest_test.h Outdated
#define ADBG_EXPECT_TEEC_SUCCESS(c, got) \
ADBG_EXPECT_ENUM(c, TEEC_SUCCESS, got, ADBG_EnumTable_TEEC_Result)

#define ADBG_EXPECT_TEEC_SUCCESS_OR_NOTSUPPORT(c, got) \
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.

If we are to truncate NOT_SUPPORTED (which I think is a good idea), I'd rather have ADBG_EXPECT_TEEC_SUCCESS_OR_NOTSUPP. I think it reads better, and also think about EOPNOTSUPP.

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.

I have my doubts about the macro. Is this hard to read or write?

res = TEEC_InvokeCommand(...);
if (res == TEE_ERROR_NOT_SUPPORTED)
        Do_ADBG_Log("PTA_INVOKE_TESTS_CMD_TRANSFER_LIST_TESTS not supported, skipping");
else
        ADBG_EXPECT_TEEC_SUCCESS(c, res);

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.

I'm fine with that too, it's what we have been doing for ages. With the advantage that the message can be more detailed.

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.

I would also be in favor of Jens' explicit implementation.

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.

The idea of my patch is just to respect and follow the existing pattern, if all of you agree I can change it to what @jenswi-linaro proposed.

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.

@raymo200915
Copy link
Copy Markdown
Contributor Author

All comments are addressed.
All commits are squashed and rebased.

@jenswikl
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

Copy link
Copy Markdown
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>

Add test entry for transfer list in xtest 1001.
Minor improvements for the code pattern.

Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
@raymo200915
Copy link
Copy Markdown
Contributor Author

Added tags and rebased.

@jforissier jforissier merged commit 658859f into OP-TEE:master May 6, 2025
1 check passed
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.

4 participants