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

Check that all configuration options are tested in all.sh #3417

Closed
gilles-peskine-arm opened this issue Jun 11, 2020 · 2 comments
Closed

Check that all configuration options are tested in all.sh #3417

gilles-peskine-arm opened this issue Jun 11, 2020 · 2 comments
Assignees
Labels
component-test Test framework and CI scripts enhancement size-s Estimated task size: small (~2d)

Comments

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Jun 11, 2020

all.sh runs build+test cycles with various configurations. We can't test all combinations of config.h symbols, but we should test all the relevant ones.

The goal of this issue is a step along the way: for every configuration option in config.h, check whether there is at least one build+test run with it enabled and one with it disabled. The runs can be as a component in all.sh, or via a depends-xxx script. The minimum deliverable to fulfill this issue is a list of configuration options that aren't covered both ways. Ideally this list should be checked mechanically.

For example, any option that is off by default but enabled in full is already covered both ways. Any option that is on by default but disabled in a depends run is already covered both ways. Any option that is disabled in full and not covered by a separate component in all.sh is not covered on. Any option that is enabled by default and never disabled in any test run is not covered off.

When working on this issue, it would be useful to make a pass of reviewing all.sh components, and check if some of them may be fully or partially redundant. For example, component_test_variable_ssl_in_out_buffer_len_CID is mostly redundant since it's default plus options that are enabled in full.

In other words, this issue is about enabling and disabling options individually. Some combinations of options need to be tested, but that's out of scope. To go further, cppcheck may help with figuring out which option combinations lead to different code in a module. Measuring code coverage could also help (ideally, all code lines should be built at some point, although realistically some platform-specific lines and aren't going to run on our CI ever).

Related: #2691 and #2002 — but they're about making sure that all test cases are executed. This is about trying out configurations that may not have specific test cases (they may be tested through test code that works whether or not these options are enabled).

Definition of done: if an option is never enabled or never disabled in all.sh, this causes a CI failure. There may be exceptions; legitimate exceptions (e.g. platform-specific settings for platforms we don't test) must be whitelisted with an explanation, and non-legitimate exceptions (we should test XXX but we don't) may be whitelisted with a link to a GitHub issue requesting extra test coverage.

@gilles-peskine-arm gilles-peskine-arm changed the title Check that all options are tested in all.sh Check that all configuration options are tested in all.sh Jun 11, 2020
@gilles-peskine-arm gilles-peskine-arm added size-m Estimated task size: medium (~1w) component-test Test framework and CI scripts labels Oct 17, 2023
@gilles-peskine-arm
Copy link
Contributor Author

I just thought of an easier way to do it: we can piggyback on the existing mechanism for testing that all test cases have been executed during outcome analysis. For each configuration option, automatically emit two test cases

Config MBEDTLS_FOO enabled
depends_on:MBEDTLS_FOO
pass

Config MBEDTLS_FOO disabled
depends_on:!MBEDTLS_FOO
pass

where pass is a function that always passes. If we aren't testing with a certain configuration option, that means one of these test cases is not executed.

We already have scripts that enumerate all configuration options (config.py, generate_query_config.pl) and a general framework for automatically generating test cases, so this would be easy to implement.

@gilles-peskine-arm gilles-peskine-arm added size-s Estimated task size: small (~2d) and removed size-m Estimated task size: medium (~1w) labels Apr 11, 2024
@gilles-peskine-arm gilles-peskine-arm self-assigned this May 23, 2024
@minosgalanakis minosgalanakis moved this to Test cases not executed in Mbed TLS Epics Aug 2, 2024
@gilles-peskine-arm
Copy link
Contributor Author

This will be handled by the combination of #9172 (already merged) to add the reporting, and #9593 (work in progress) to change the report from a warning to an error. Because the work that is specific to this issue has been merged, I am closing this issue as completed.

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-s Estimated task size: small (~2d)
Projects
Status: Done
Development

No branches or pull requests

1 participant