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

Guest test initial commit #118

Merged
merged 8 commits into from
Oct 13, 2023
Merged

Guest test initial commit #118

merged 8 commits into from
Oct 13, 2023

Conversation

hongyuni
Copy link
Contributor

fix a path error in test_executor

@ysun ysun requested a review from hanning0511 September 28, 2023 03:51
@hongyuni
Copy link
Contributor Author

hongyuni commented Sep 28, 2023

@hongyuni
Copy link
Contributor Author

hello, Ning, Yi, any process regarding this code review?

@ysun
Copy link
Contributor

ysun commented Oct 10, 2023

The patch set looks good to me!
Thanks Hongyu.

@ysun
Copy link
Contributor

ysun commented Oct 10, 2023

I'm not pretty sure whether we should care about the coding check

    guest-test/guest.test_executor.sh:28 - Double quote to prevent globbing and word splitting., refer to https://github.com/koalaman/shellcheck/wiki/SC2086
    guest-test/guest.test_executor.sh:34 - Double quote to prevent globbing and word splitting., refer to https://github.com/koalaman/shellcheck/wiki/SC2086
    guest-test/guest.test_launcher.sh:76 - Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181
    guest-test/guest.test_launcher.sh:199 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053
    guest-test/guest.test_launcher.sh:203 - Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181
    guest-test/guest.test_launcher.sh:274 - Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181
    guest-test/guest.test_launcher.sh:291 - Check exit code directly with e.g. 'if mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181
    guest-test/guest.test_launcher.sh:3[11](https://github.com/intel/lkvs/actions/runs/6335452419/job/17565972141?pr=118#step:4:12) - Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181

Ning can give some comments.

@hongyuni
Copy link
Contributor Author

hongyuni commented Oct 10, 2023

[Hongyu] $1 not double quoted here, is it quite necessary?

guest-test/guest.test_executor.sh:28 - Double quote to prevent globbing and word splitting., refer to https://github.com/koalaman/shellcheck/wiki/SC2086
guest-test/guest.test_executor.sh:34 - Double quote to prevent globbing and word splitting., refer to https://github.com/koalaman/shellcheck/wiki/SC2086

[Hongyu] use $? to make rest of logic clear reading, if !mycmd may confuse and not easy to maintain in future

    guest-test/guest.test_launcher.sh:76 - Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181
    guest-test/guest.test_launcher.sh:203 - Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181
    guest-test/guest.test_launcher.sh:274 - Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181
    guest-test/guest.test_launcher.sh:291 - Check exit code directly with e.g. 'if mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181
    guest-test/guest.test_launcher.sh:3[11](https://github.com/intel/lkvs/actions/runs/6335452419/job/17565972141?pr=118#step:4:12) - Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?., refer to https://github.com/koalaman/shellcheck/wiki/SC2181

[Hongyu] this one is tricky and weird, tried revise it but the logic can't work as expected, so keep it the way right now
guest-test/guest.test_launcher.sh:199 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053

@hongyuni hongyuni force-pushed the guest_test_tdx_bugfix branch from fa90e30 to 2140c13 Compare October 12, 2023 12:21
@hongyuni
Copy link
Contributor Author

Hi @hanning0511 , @ysun

I've revised based on offline review with Ning, the remained code check failure like following will be ignored temporarily since code change directly will broke the logic unexpectedly.

guest-test/guest.test_launcher.sh:199 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053

please note, there are 5 more code check failure like above brought back to secure the original design logic.

as synced with Ning, will seek solution continuously and try to fix it with no code check failure + all logic correct.

would you help to review and decide whether code can be merged?

Thanks,
Hongyu

@ysun
Copy link
Contributor

ysun commented Oct 13, 2023

Hi @hanning0511 , @ysun

I've revised based on offline review with Ning, the remained code check failure like following will be ignored temporarily since code change directly will broke the logic unexpectedly.

guest-test/guest.test_launcher.sh:199 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053

please note, there are 5 more code check failure like above brought back to secure the original design logic.

as synced with Ning, will seek solution continuously and try to fix it with no code check failure + all logic correct.

would you help to review and decide whether code can be merged?

Thanks, Hongyu

That works for me. I've noticed that you've made a new commit to address the issue. However, it seems that another "merge commit" has been added, which has made the commit history somewhat cluttered. Would it be possible for you to remove that merge commit to keep the history cleaner?
You can just operate on your tree(run a git rebase), then the PR will be updated automatically.

I'd like merge it today, so long as Ning has no comments.
@hanning0511 Please "approve it" if the patch set looks good to you.

add guest-test framework folder and description

Signed-off-by: Hongyu Ning <[email protected]>
test_launcher script initial commit

Signed-off-by: Hongyu Ning <[email protected]>
qemu.config.json template initial commit, parse script added along to
match it

Signed-off-by: Hongyu Ning <[email protected]>
qemu_runner script to launch guest vm based on qemu.config and script
parameters args override

Signed-off-by: Hongyu Ning <[email protected]>
test_executor includes basic framework to prepare and run tests in guest
vm based on $TESTCASE

Signed-off-by: Hongyu Ning <[email protected]>
please refer to this implmentation to add more guest vm testcases

Signed-off-by: Hongyu Ning <[email protected]>
fix path error in test_executor

Signed-off-by: Hongyu Ning <[email protected]>
code style improvement based on github code check, plus minor json
contents change and qemu_get_config.py print context change

Signed-off-by: Hongyu Ning <[email protected]>
@hongyuni hongyuni force-pushed the guest_test_tdx_bugfix branch from 2140c13 to 2d404c0 Compare October 13, 2023 01:56
@ysun
Copy link
Contributor

ysun commented Oct 13, 2023

Currently, there is only one type of Check that is being ignored. However, we can work on resolving it in the future.

guest-test/guest.test_launcher.sh:211 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053
guest-test/guest.test_launcher.sh:216 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053
guest-test/guest.test_launcher.sh:219 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053
guest-test/guest.test_launcher.sh:222 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053
guest-test/guest.test_launcher.sh:225 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053
guest-test/guest.test_launcher.sh:228 - Quote the right-hand side of == in [[ ]] to prevent glob matching., refer to https://github.com/koalaman/shellcheck/wiki/SC2053

@ysun ysun merged commit 3107de6 into intel:main Oct 13, 2023
1 of 2 checks passed
@hongyuni hongyuni changed the title Guest test tdx bugfix Guest test initial commit Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants