Skip to content

QAT, e2e: add qat-engine's testapp case #1885

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 30, 2024
Merged

QAT, e2e: add qat-engine's testapp case #1885

merged 2 commits into from
Oct 30, 2024

Conversation

hj-johannes-lee
Copy link
Contributor

@hj-johannes-lee hj-johannes-lee commented Oct 18, 2024 •
edited by mythi
Loading

Fixes: #1687

@hj-johannes-lee hj-johannes-lee marked this pull request as draft October 18, 2024 03:12
mythi
mythi reviewed Oct 18, 2024
View reviewed changes
@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-011 branch 2 times, most recently from 142ec19 to 693c654 Compare October 18, 2024 13:17
mythi
mythi reviewed Oct 18, 2024
View reviewed changes
@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-011 branch 3 times, most recently from c9dc7dc to cd20b26 Compare October 18, 2024 13:43
mythi
mythi reviewed Oct 18, 2024
View reviewed changes
@hj-johannes-lee
Copy link
Contributor Author

hj-johannes-lee commented Oct 18, 2024 •
edited
Loading

e2e-qat fails in the runner multiple times though it runs in my env. I think 30 seconds for waiting for the resources is too short.
Let me try with 60s.

@mythi
Copy link
Contributor

mythi commented Oct 21, 2024

e2e-qat fails in the runner multiple times though it runs in my env. I think 30 seconds for waiting for the resources is too short. Let me try with 60s.

this new test can be skipped on e2e-qat-gnrd.

@hj-johannes-lee
Copy link
Contributor Author

this new test can be skipped on e2e-qat-gnrd.

Changed back to 30s and disabled the test in e2e-qat-gnrd

mythi
mythi reviewed Oct 21, 2024
View reviewed changes
@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-011 branch 2 times, most recently from 18a9eec to 9e2cb26 Compare October 21, 2024 15:17
mythi
mythi reviewed Oct 22, 2024
View reviewed changes
@hj-johannes-lee
Copy link
Contributor Author

hj-johannes-lee commented Oct 23, 2024 •
edited
Loading

@mythi no-install-recommends makes building fail. For making it build without fail, we need to add more.
Is it what we want? They are anyway in builder image that will not be included in the final image.

edit:
I guess ca-certificates and automake need to be added.

@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-011 branch 3 times, most recently from 910efc4 to a2953ca Compare October 23, 2024 04:11
@eero-t
Copy link
Contributor

eero-t commented Oct 23, 2024 •
edited
Loading

@mythi no-install-recommends makes building fail. For making it build without fail, we need to add more. Is it what we want? They are anyway in builder image that will not be included in the final image.

The point of no-install-recommends is that one specifies the packages that are actually needed, and avoids "nice to have" (recommended) extras, as those can bring lot of unintended stuff through dependency chain. Mostly it's to keep layer size minimal / speed up builds, but for final image minimizing BOM also avoids getting unnecessary CVE notifications.

@hj-johannes-lee hj-johannes-lee marked this pull request as ready for review October 23, 2024 14:02
mythi
mythi reviewed Oct 23, 2024
View reviewed changes
Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

looks like some copy-paste errors to fix but looking good already

@hj-johannes-lee
Copy link
Contributor Author

hj-johannes-lee commented Oct 23, 2024 •
edited
Loading

@eero-t Oho.! Thank you so much for the detailed explanation.! :) The matter of getting unnecessary CVE notifications is somethin that I have not even able to think.!

@hj-johannes-lee hj-johannes-lee force-pushed the PR-2024-011 branch 2 times, most recently from e186b28 to 216db00 Compare October 28, 2024 12:49
mythi
mythi approved these changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@mythi mythi left a comment

Choose a reason for hiding this comment

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

LGTM

@hj-johannes-lee
QAT: add testapp of qatengine
7af23f1 
Signed-off-by: Hyeongju Johannes Lee <[email protected]>
@hj-johannes-lee
QAT, e2e: add qat-engine's testapp case
bdc86bc 
Add qat-engine testapp e2e tests

Disable in CI/CD since the test is known not to work on simics.

Signed-off-by: Hyeongju Johannes Lee <[email protected]>
@mythi mythi merged commit 6667255 into intel:main Oct 30, 2024
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mythi mythi mythi approved these changes

@bart0sh bart0sh Awaiting requested review from bart0sh bart0sh is a code owner

@ozhuraki ozhuraki Awaiting requested review from ozhuraki ozhuraki is a code owner

@tkatila tkatila Awaiting requested review from tkatila tkatila is a code owner

@kad kad Awaiting requested review from kad kad is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

QAT testing/deployment updates
3 participants
@hj-johannes-lee @mythi @eero-t