Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test framework investigation #8223

Closed
ronald-cron-arm opened this issue Sep 19, 2023 · 6 comments
Closed

Test framework investigation #8223

ronald-cron-arm opened this issue Sep 19, 2023 · 6 comments
Assignees
Labels
component-test Test framework and CI scripts enhancement size-m Estimated task size: medium (~1w)

Comments

@ronald-cron-arm
Copy link
Contributor

ronald-cron-arm commented Sep 19, 2023

WORK IN PROGRESS

Goal: Move into a repo different from Mbed-TLS and TF-PSA-Crypto, the test/infrastructure code common to Mbed TLS and TF-PSA-Crypto, so called the test framework to avoid file duplication. The test framework code is not tight to a particular version of
Mbed TLS or TF-PSA-Crypto and should thus work across all supported branches of the two repos.

Files from the scripts directory:

CI F C S Issue/PR Comment
abi_check.py y y y(1.x) S CI scripts
apidoc_full.sh y y y S called by doxygen.sh, depends on config.py, needs changes for TF-PSA-Crypto
assemble_changelog.py y y y XS 47 adapted to TF-PSA-Crypto
code_style.py y y y XS 48
generate_features.pl + generate_query_config.pl y y y M adapt to mbedtls_config.h split, adapt to tf_psa_crypto_config.h
lcov.sh y y y XS 41 adapted to TF-PSA-Crypto
min_requirements.py y y y XS CI scripts
output_env.sh y y y XS Called by all.sh, basic-build-test.sh
generate_psa_constants.py y y y XS Called by builds, check-generated-files.sh
generate_ssl_debug_helpers.py y y n XS Called by builds, check-generated-files.sh
generate_driver_wrappers.py y n y - branch specific
generate_errors.pl y n n - Error scheme expected to evolve significantly in the short term
generate_visualc_files.pl y(4.x?) n n
make_generated_files.bat y(4.x?) n n
windows_msbuild.bat y(4.x?) n n
basic.requirements.txt y n y - branch specific
ci.requirements.txt y n y - branch specific
driver.requirements.txt y n y - branch specific
maintainer.requirements.txt y n y - branch specific
config.pl y(4.x?) n n - Kept in Mbed TLS for backward compatibility
config.py y n y - branch specific
bump_version.sh n y y S adapt to TF-PSA-Crypto
code_size_compare.py n y y M - adapt to mbedtls_config.h split, adapt to tf_psa_crypto_config.h
ecc-heap.sh n y y S depends on benchmark.c, adapt to tf_psa_crypto_config.h
ecp_comb_table.py n n y - Utility to add a new curve in ecp_curves.c, tight to ecp_curves.c code
footprint.sh n y n do we keep it? broken, redundant with code_size_compare.py?
memory.sh + massif_max.pl n y n do we keep it? if yes, adapt to mbedtls_config.h split
prepare_release.sh n y y ? do we keep it?
tmp_ignore_makefiles.sh n y y S do we keep it? if yes, adapt to TF-PSA-Crypto tree

Proposal: The test framework is made of:
tests/scripts/all.sh (not the components), analyze_outcomes.py, generate_test_code.py, test_generate_test_code.py

tests/suites/helpers.function, main_test.function, host_test.function
tests/include/test/arguments.h, helpers.h, macros.h, random.h, psa_crypto_helpers.h(?, partly ?)
tests/src/helpers.c, random.c, psa_crypto_helpers.c(?, partly?)

The test framework code aimed to be integrated into 3.6, 4.x and TF-PSA-Crypto which have different configuration options thus as a first approximation the test framework code should be independent of configuration options (this is not the case for some of it currently).

Some PSA crypto test helpers are of value to TLS test code like mbedtls_test_fail_if_psa_leaking() and associated macros.

What is the development flow for the test framework code?
Local development in Mbed TLS and/or TF-PSA-Crypto to be able to test it locally, then pull request against the development branch of the test framework repo where the code review happens + push of the local work on Mbed TLS and/or TF-PSA-Crypto for the benefit of code reviewers. The pull request is tested by the CI against the head of the Mbed TLS and TF-PSA-Crypto supported branches. Once the PR is merged in the test framework repo, update Mbed TLS and TF-PSA-Crypto supported branches accordingly: should be automated.

Heavy development flow, we probably do not want to execute it too often.
Profile of eligible files:

  • quite stable, not changed often
  • shared between Mbed TLS and TF-PSA-Crypto (not a MUST rather a plus )
  • weakly version dependent, not version dependent ?

About infrastructure/test file that depends on version dependent code in the test framework:

  • If the version dependent code changes often, this does not seem to be viable: the development flow is significantly more complicated than with a version dependent file, likely that the file will become quite rapidly hard to maintain. Example: test suites.
  • For any such a file, the changes of major versions are critical as many things that were taken as granted can change.
  • Promote keeping code common which should lead to better thought/structured code
@ronald-cron-arm ronald-cron-arm added enhancement size-m Estimated task size: medium (~1w) component-test Test framework and CI scripts labels Sep 19, 2023
@ronald-cron-arm ronald-cron-arm self-assigned this Sep 19, 2023
@gilles-peskine-arm
Copy link
Contributor

I don't understand how you've come to this particular list of files. I'd expect most of tests/** and parts of scripts/** to move to the new version-agnostic repository.

@ronald-cron-arm
Copy link
Contributor Author

ronald-cron-arm commented Sep 20, 2023

I don't understand how you've come to this particular list of files. I'd expect most of tests/** and parts of scripts/** to move to the new version-agnostic repository.

Roughly that's the list of test files that to me are used in both crypto and TLS testing. Otherwise even though I mentioned 3.6 I did not realize the impact of that: version-agnostic test framework.

Could you elaborate what you have in mind when you say "I'd expect most of tests/** ... to move to the new version-agnostic repository? Especially do you have in mind that the test suites should be part of it, test_suite.*.data/function files?

More generally has this idea of version-agnostic test framework been discussed, documented somewhere?

@gilles-peskine-arm
Copy link
Contributor

I'd expect most of tests/**

Oops, I forgot to write the end of that clause: most of tests/** _except for test_suite_*.

In particular, I'd expect all the test and maintainer scripts to move (for some of them, with a data file getting separated out), as well as all of tests/src/**.

More generally has this idea of version-agnostic test framework been discussed, documented somewhere?

I think we've only discussed it orally (and most of the discussion probably happened between the two of us). A lot of the planning is going to be on a file-by-file basis, so if we want to plan the time, we need time to do some analysis, and I believe we'll finally have time next quarter. Although I think the most effective way, in terms of total time spent, would be to not do a detailed analysis and instead directly do the work, file by file, moving things to different directories until we have a directory with version-agnostic files.

@ronald-cron-arm
Copy link
Contributor Author

In particular, I'd expect all the test and maintainer scripts to move (for some of them, with a data file getting separated out), as well as all of tests/src/**.

I am quite skeptical about some files in tests/src/ regarding version independence. Some use internal APIs, structures that can be changed at any time (like bignum_helpers.c), some use TLS or crypto public APIs that can be changed when we change the major version and we are going to do it soon. As far as I know there has been no specific constraints on these files: they can use any internal or public TLS or crypto API or type. This does not seem to be compatible with being version independent. What am I missing?

@gilles-peskine-arm
Copy link
Contributor

I expect the test helper code to have some ifdefs on internal macros in addition to public macros. (It already does!) Sometimes if we change an internal interface we might want to create an internal macro just for the sake of the test code. That's the cost of having separate repositories, and it's not a very high cost. Certainly a lower cost than maintaining a lot of quasi-duplicate code that we spend a lot of effort re-synchronizing, which we're doing now between development and the LTS branches.

@ronald-cron-arm
Copy link
Contributor Author

We have a design document for the framework directory now, we know which file we want to move to it and we have estimated the time needed. This investigation issue can be closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-test Test framework and CI scripts enhancement size-m Estimated task size: medium (~1w)
Projects
Status: Framework 1/3 (MVP repo split)
Development

No branches or pull requests

2 participants