-
Notifications
You must be signed in to change notification settings - Fork 15
ci: enhancements #108
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
ci: enhancements #108
Conversation
Summary of ChangesHello @mmartinv, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant improvements to the CI/testing infrastructure, primarily by refactoring certificate generation utilities and enhancing service configuration capabilities. The changes aim to provide more robust support for HTTPS across various FDO services within the testing environment, improve error reporting, and streamline the setup process for both standard and RPM-based CI tests. These updates contribute to a more reliable and maintainable testing suite. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces significant refactoring to the CI test scripts, primarily to add support for HTTPS and improve modularity by reducing code duplication. The changes are well-structured, centralizing common logic into utility scripts. I've identified a few critical issues related to shell quoting within sudo bash -c blocks in the FMF test utilities, which will prevent the HTTPS configuration from being applied correctly. Additionally, I've suggested a couple of robustness improvements for the trap handlers to prevent potential recursive calls. Overall, this is a great improvement to the test suite.
0d9d42b to
5b671ab
Compare
5b671ab to
a3edf70
Compare
5f939ea to
210b757
Compare
38ce1bd to
44832ef
Compare
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant and valuable set of enhancements to the CI/CD infrastructure. The changes primarily focus on refactoring and improving the test scripts, resulting in better code reuse, maintainability, and robustness across different testing environments (local CI, containers, and FMF/RPM). Key improvements include modernizing the Packit configuration, enhancing the Makefile for more flexible local builds, and restructuring the shell scripts for better modularity.
My review identifies a few areas for improvement: a potential regression in file permissions for generated certificates which could impact service startup, a suggestion to enhance failure reporting in CI tests by including logs, a minor typo fix in a log message, and a proposed optimization for log collection in containerized tests to avoid redundancy. Overall, these are excellent changes that will improve the development and testing workflow.
f63e443 to
a3588fe
Compare
a3588fe to
7932647
Compare
kgiusti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
pcdubs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Miguel Martín <[email protected]>
Move the pub key generation part to a different function Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
b41732c to
e74ee7c
Compare
Signed-off-by: Miguel Martín <[email protected]> Co-authored-by: Paul Whalen <[email protected]>
Source `utils.sh` instead of `test-onboarding.sh` because the `run_test` is being fully overwritten Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
e74ee7c to
e62aee8
Compare

No description provided.