QNX8 QEMU x86_64 Integration tests#19
QNX8 QEMU x86_64 Integration tests#19TimofteBogdan1900 wants to merge 47 commits intoeclipse-score:mainfrom
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
4c6214f to
b5c0d24
Compare
d41ded4 to
a7b9a0e
Compare
a7b9a0e to
4a6f50e
Compare
Co-authored-by: Jorge C. Santos <156771132+jorgecasal@users.noreply.github.com> Signed-off-by: TimofteBogdan1900 <bogdan.timofte@etas.com>
23e7bf2 to
0cee3e4
Compare
NEOatNHNG
left a comment
There was a problem hiding this comment.
Reviewed the general setup etc. Still need to review the integration test setup itself.
deployment/qemu/configs/BUILD.bazel
Outdated
| "qcrypto.conf", | ||
| "startup.sh", | ||
| ], | ||
| visibility = ["//visibility:public"], |
There was a problem hiding this comment.
Public visibility probably a bit much. We only want to share it within our module right?
| visibility = ["//visibility:public"], | |
| visibility = ["//:__subpackages__"], |
deployment/qemu/BUILD.bazel
Outdated
| [ | ||
| "run_qemu.sh", | ||
| ], | ||
| visibility = ["//visibility:public"], |
| args = [ | ||
| "-config_file", | ||
| "$(rootpath :config_file)", | ||
| "-service_instance_manifest", | ||
| "$(rootpath etc/mw_com_config.json)", | ||
| ], | ||
| data = [ | ||
| "etc/mw_com_config.json", | ||
| ":config_file", | ||
| ], |
There was a problem hiding this comment.
The "gatewayd" binary shall only include the stuff that is always necessary and no testing-specific stuff. Everything that you need to add for testing please create a native_binary like the "gatewayd_example" below.
There was a problem hiding this comment.
i have not added any testing specific . the same as before param path to bin file and mw::com json file are passed
| // Read config data | ||
| // TODO: Be more flexible with the path | ||
| // TODO: Use memory mapped file instead of copying into buffer | ||
| std::string config_path = "src/gatewayd/etc/gatewayd_config.bin"; |
There was a problem hiding this comment.
Use zero-terminated zstring_view from baselibs
| std::string config_path = "src/gatewayd/etc/gatewayd_config.bin"; | |
| score::safecpp::zstring_view config_path = "src/gatewayd/etc/gatewayd_config.bin"; |
There was a problem hiding this comment.
i think is better to remove this hardcoded string
std::string config_path = "src/gatewayd/etc/gatewayd_config.bin";
and rely on the method so that the path to the config file is configurable
What do you think ?
src/someipd/BUILD.bazel
Outdated
| # Export config files for deployment | ||
| exports_files( | ||
| ["etc/mw_com_config.json"], | ||
| visibility = ["//visibility:public"], |
.devcontainer/devcontainer.json
Outdated
| } | ||
| } | ||
| }, | ||
| "postCreateCommand": "sudo apt-get update && sudo apt-get install -y software-properties-common && sudo add-apt-repository -y universe && sudo apt-get update && sudo apt-get install -y qemu-system-x86 iputils-ping tcpdump iptables", |
There was a problem hiding this comment.
Should we add that to the Dockerfile instead? Then it would be one step.
Otherwise the Docker image will be generated and then afterwards this additional stuff is installed.
Also we are installing way too much stuff (icon themes etc.). I guess you should add --no-install-recommends
| "postCreateCommand": "sudo apt-get update && sudo apt-get install -y software-properties-common && sudo add-apt-repository -y universe && sudo apt-get update && sudo apt-get install -y qemu-system-x86 iputils-ping tcpdump iptables", | |
| "postCreateCommand": "sudo apt-get update && sudo apt-get install -y --no-install-recommends software-properties-common && sudo add-apt-repository -y universe && sudo apt-get update && sudo apt-get install -y --no-install-recommends qemu-system-x86 iputils-ping tcpdump iptables", |
There was a problem hiding this comment.
updated
README.md
Outdated
| It is stronly recommended to run all tests with `--nocache_test_results` which is the best way on development cycles to ensure you are always running the latest version of the tests and not accidentally running cached results. | ||
|
|
||
| Build the docker containers: | ||
| Tu run all tests (will take around 2 minutes) |
There was a problem hiding this comment.
| Tu run all tests (will take around 2 minutes) | |
| To run all tests (will take around 2 minutes): |
README.md
Outdated
| As a proof of concept `docker compose` can be used to build, setup and run the containers. | ||
| In the future a pytest based setup can be implemented to orchestrate the containers. | ||
| For the QEMU QNX x864 image to run on host please run the script deployment/qemu/setup_bridge.sh with sudo privileges to setup the required network bridge and tap interfaces. | ||
| It is stronly recommended to run all tests with `--nocache_test_results` which is the best way on development cycles to ensure you are always running the latest version of the tests and not accidentally running cached results. |
There was a problem hiding this comment.
| It is stronly recommended to run all tests with `--nocache_test_results` which is the best way on development cycles to ensure you are always running the latest version of the tests and not accidentally running cached results. | |
| It is recommended to run all tests with `--nocache_test_results` to ensure you are always running the latest version of the tests and not accidentally seeing cached results. |
README.md
Outdated
| ``` | ||
|
|
||
| Finally start the benchmark on the someipd-1 container in a third shell: | ||
| To run the QNX tests using dev containers please execute the shell command: |
There was a problem hiding this comment.
This is only needed in devcontainer or also when running locally?
And why don't we add it to the devcontainer file so that it gets executed automatically? Is there a negative impact?
There was a problem hiding this comment.
First question : is needed independent if its host or docker containers .
Second question:
Done
Features
QNX QEMU ARM image creation is not possible due to tool missing startup-qemu-virt from the qnx SDP.
How to run tests see:
See Readme.md chapter