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

Add meson option to disable explicit EC keys tests #492

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

Mulling
Copy link
Contributor

@Mulling Mulling commented Dec 26, 2024

Description

This PR adds a meson option to disable explicit EC keys tests.

Checklist

  • Code modified for feature
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated

Reviewer's checklist:

  • Any issues marked for closing are addressed
  • There is a test suite reasonably covering new functionality or modifications
  • This feature/change has adequate documentation added
  • Code conform to coding style that today cannot yet be enforced via the check style test
  • Commits have short titles and sensible commit messages
  • Coverity Scan has run if needed (code PR) and no new defects were found

@Mulling Mulling changed the title Add option to disable explicit EC keys tests Add meson option to disable explicit EC keys tests Dec 26, 2024
@Jakuje
Copy link
Contributor

Jakuje commented Dec 26, 2024

Should we make it the other way round and disable the explicit curves by default and enable them only if some configuration option is provided instead?

@Mulling
Copy link
Contributor Author

Mulling commented Dec 26, 2024

Would be better, I think debian is the only one that allows it.

@Mulling
Copy link
Contributor Author

Mulling commented Dec 27, 2024

@Jakuje Not sure how you want to handle the workflow config, I just copied what was already done for preload_libasan.

Jakuje
Jakuje previously approved these changes Dec 28, 2024
Copy link
Contributor

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

just one nit, otherwise looks good!

tests/tecxc Outdated Show resolved Hide resolved
Copy link
Contributor

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

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

lgtm. Thank you for your contribution!

@simo5
Copy link
Member

simo5 commented Jan 6, 2025

@Mulling can you please squash the commits and rebase on main?

@simo5 simo5 added the covscan-ok Coverity scan passed label Jan 6, 2025
@Mulling Mulling force-pushed the main branch 2 times, most recently from 3636429 to 4ab95c5 Compare January 6, 2025 17:25
* Disable explicit EC keys tests by default

Signed-off-by: Lucas Mulling <lucas.mulling@suse.com>
@Mulling
Copy link
Contributor Author

Mulling commented Jan 7, 2025

Not sure why Coverity Scan is failing.

@simo5
Copy link
Member

simo5 commented Jan 7, 2025

Not sure why Coverity Scan is failing.

Don't worry about it, we'll handle it later, it is just telling that actual code has changed and a scan should be rerun (it is not automatically run for "reasons").

@simo5 simo5 added covscan-ok Coverity scan passed and removed covscan-ok Coverity scan passed labels Jan 7, 2025
Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

LGTM

@simo5 simo5 merged commit 8541930 into latchset:main Jan 7, 2025
41 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
covscan-ok Coverity scan passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants