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

tests: add a test for pkg-config files #8988

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

billatarm
Copy link
Contributor

@billatarm billatarm commented Mar 25, 2024

tests: add a test for pkg-config files

Add a test that does some basic validation of the pkg-config files.

Example run:

./tests/scripts/all.sh test_cmake_as_package
******************************************************************
* test_cmake_as_package: build: cmake 'as-package' build
* Mon Apr 29 12:33:32 PM CDT 2024
******************************************************************
cmake .
make
Built against Mbed TLS 3.5.2
testing package config file: mbedtls ... passed
testing package config file: mbedx509 ... passed
testing package config file: mbedcrypto ... passed
make clean

Signed-off-by: Bill Roberts bill.roberts@arm.com

Description

Please write a few sentences describing the overall goals of the pull request's commits.

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

@billatarm
Copy link
Contributor Author

Ill do a back port for 2.28 once the reviewers are happy with this PR.

@billatarm
Copy link
Contributor Author

@billatarm
Copy link
Contributor Author

Does the CI loop just run the all.sh script or is their anything I need to do to get the CI to pick this up?

@paul-elliott-arm paul-elliott-arm self-assigned this Mar 27, 2024
@paul-elliott-arm paul-elliott-arm added enhancement needs-review Every commit must be reviewed by at least two team members, needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review size-s Estimated task size: small (~2d) component-test Test framework and CI scripts priority-high High priority - will be reviewed soon labels Mar 27, 2024
tests/scripts/all.sh Outdated Show resolved Hide resolved
@gilles-peskine-arm
Copy link
Contributor

FYI the CI is still failing.

@gilles-peskine-arm gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Apr 8, 2024
@billatarm billatarm force-pushed the add-pc-files branch 2 times, most recently from 0ff15a1 to a33fc7c Compare April 29, 2024 17:36
tests/CMakeLists.txt Outdated Show resolved Hide resolved
tests/Descriptions.txt Outdated Show resolved Hide resolved
tests/scripts/all.sh Outdated Show resolved Hide resolved
@billatarm
Copy link
Contributor Author

FYI the CI is still failing.

The current CI is just showing orange, is it in some long queue or how do we kick that off?

@tom-cosgrove-arm
Copy link
Contributor

The current CI is just showing orange, is it in some long queue or how do we kick that off?

I've kicked the CI - https://mbedtls.trustedfirmware.org/job/mbed-tls-pr-head/job/PR-8988-head/ - it won't run automatically on PRs from non-team members, sorry

@gilles-peskine-arm
Copy link
Contributor

CI triggered

Add a test that does some basic validation of the pkg-config files.

Example run:
./tests/scripts/all.sh test_cmake_as_package
******************************************************************
* test_cmake_as_package: build: cmake 'as-package' build
* Mon Apr 29 12:33:32 PM CDT 2024
******************************************************************
cmake .
make
Built against Mbed TLS 3.5.2
testing package config file: mbedtls ... passed
testing package config file: mbedx509 ... passed
testing package config file: mbedcrypto ... passed
make clean

Signed-off-by: Bill Roberts <bill.roberts@arm.com>
Copy link
Member

@paul-elliott-arm paul-elliott-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@billatarm
Copy link
Contributor Author

LGTM

I'll backport this to 2.28 now

@paul-elliott-arm
Copy link
Member

LGTM

I'll backport this to 2.28 now

And unfortunately 3.6, although that should be utterly trivial given the lack of differences there.

@billatarm
Copy link
Contributor Author

LGTM

I'll backport this to 2.28 now

And unfortunately 3.6, although that should be utterly trivial given the lack of differences there.

3.6 backport is here: #9174

@paul-elliott-arm paul-elliott-arm removed the needs-ci Needs to pass CI tests label May 23, 2024
@billatarm
Copy link
Contributor Author

LGTM

I'll backport this to 2.28 now

And unfortunately 3.6, although that should be utterly trivial given the lack of differences there.

3.6 backport is here: #9174

and 2.28: #9175

Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@tom-cosgrove-arm tom-cosgrove-arm added approved Design and code approved - may be waiting for CI or backports needs-backports Backports are missing or are pending review and approval. labels May 25, 2024
@gilles-peskine-arm
Copy link
Contributor

@tom-cosgrove-arm Did you mean to approve? You left a review comment that says “LGTM” but you didn't approve as defined by GitHub.

@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, and removed approved Design and code approved - may be waiting for CI or backports labels May 29, 2024
billatarm added a commit to billatarm/mbedtls that referenced this pull request May 29, 2024
Add a test that does some basic validation of the pkg-config files.

Note: this is a port of Mbed-TLS#8988
but is moved into a different test component so cmake shared
infrastucture isn't needed.

Example run:
./tests/scripts/all.sh test_cmake_shared
******************************************************************
* test_cmake_shared: build/test: cmake shared
* Wed May 29 18:41:19 UTC 2024
******************************************************************
cmake -DUSE_SHARED_MBEDTLS_LIBRARY=On .
make
<snip>
testing package config file: mbedtls ... passed
testing package config file: mbedx509 ... passed
testing package config file: mbedcrypto ... passed
make clean

Signed-off-by: Bill Roberts <bill.roberts@arm.com>
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@tom-cosgrove-arm tom-cosgrove-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Jun 3, 2024
@tom-cosgrove-arm
Copy link
Contributor

Did you mean to approve? You left a review comment that says “LGTM” but you didn't approve as defined by GitHub.

Yes, pretty sure I clicked the "Approve" radio button, but must have miss-clicked and not noticed.

billatarm added a commit to billatarm/mbedtls that referenced this pull request Jun 4, 2024
Add a test that does some basic validation of the pkg-config files.

Note: this is a port of Mbed-TLS#8988
but is moved into a different test component so cmake shared
infrastucture isn't needed.

Note this port of the patch detects the OS and skips so things like
freebsd do not fail.

Example run:
./tests/scripts/all.sh test_cmake_shared
******************************************************************
* test_cmake_shared: build/test: cmake shared
* Wed May 29 18:41:19 UTC 2024
******************************************************************
cmake -DUSE_SHARED_MBEDTLS_LIBRARY=On .
make
<snip>
testing package config file: mbedtls ... passed
testing package config file: mbedx509 ... passed
testing package config file: mbedcrypto ... passed
make clean

Signed-off-by: Bill Roberts <bill.roberts@arm.com>
billatarm added a commit to billatarm/mbedtls that referenced this pull request Jun 7, 2024
Add a test that does some basic validation of the pkg-config files.

Note: this is a port of Mbed-TLS#8988
but is moved into a different test component so cmake shared
infrastucture isn't needed.

Note this port of the patch detects the OS and skips so things like
freebsd do not fail.

Example run:
./tests/scripts/all.sh test_cmake_shared
******************************************************************
* test_cmake_shared: build/test: cmake shared
* Wed May 29 18:41:19 UTC 2024
******************************************************************
cmake -DUSE_SHARED_MBEDTLS_LIBRARY=On .
make
<snip>
testing package config file: mbedtls ... passed
testing package config file: mbedx509 ... passed
testing package config file: mbedcrypto ... passed
make clean

Signed-off-by: Bill Roberts <bill.roberts@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-test Test framework and CI scripts enhancement needs-backports Backports are missing or are pending review and approval. priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)
Projects
Status: Has Approval
Development

Successfully merging this pull request may close these issues.

None yet

5 participants