-
Notifications
You must be signed in to change notification settings - Fork 63
Conversation
Can be rebased on master to fix the failure in |
Signed-off-by: Chekhovoi, Mykola <ext-mykola.chekhovoi@here.com>
1c5618b
to
a3b6641
Compare
Codecov Report
@@ Coverage Diff @@
## master #1458 +/- ##
=========================================
+ Coverage 80.36% 80.4% +0.04%
=========================================
Files 183 182 -1
Lines 11047 11026 -21
=========================================
- Hits 8878 8866 -12
+ Misses 2169 2160 -9
Continue to review full report at Codecov.
|
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.
Looks good, just a couple nits. I like reusing the repo generation script!
@@ -32,11 +33,18 @@ Config *Get_test_config(const char *storagePath) { | |||
config->provision.server = serverAddress; | |||
config->provision.provision_path = "tests/test_data/cred.zip"; | |||
|
|||
config->storage.path = storagePath; | |||
temp_dir = new TemporaryDirectory; |
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.
Can we use a unique_ptr
here instead of a raw new
and delete
?
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.
There is no std::make_unique in C++11.
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.
True, but we've filled in the gap:
aktualizr/src/libaktualizr/utilities/utils.h
Line 169 in d5d0c65
// this is reference implementation of make_unique which is not yet included to C++11 |
uptane_gen --command addtarget --hwid primary_hw --serial CA:FE:A6:D2:84:9D --targetname primary.txt | ||
|
||
if [ "$ADD_DEFAULT_SECONDARY" = true ] | ||
then |
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.
Not a big deal, but we usually prefer the style if [ ]; then
as you did above.
a3b6641
to
bd65a7f
Compare
I thought it looked fine but something went wrong when I tried to run this through ptest in CI: https://main.gitlab.in.here.com/olp/edge/ota/connect/client/meta-updater/-/jobs/863441 |
bd65a7f
to
215c654
Compare
…oved as unnecessary. Signed-off-by: Chekhovoi, Mykola <ext-mykola.chekhovoi@here.com>
215c654
to
9acaa0e
Compare
SET(REPO_PATH ${PROJECT_BINARY_DIR}/uptane_repos/c-api-test-repo) | ||
|
||
if(CMAKE_CROSSCOMPILING) | ||
find_program(UPTANE_GENERATOR NAMES uptane-generator) |
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.
I don't think we need this CMAKE_CROSSCOMPILING
clause. We ought to be able to do the same things we do in src/libaktualizr/primary/CMakeLists.txt
and tests/uptane_repo_generation/CMakeLists.txt
to create the repos and then make sure the test depends on that task and can find the paths.
It probably makes sense for you to run the ptests in CI directly. Basically you need to create a branch in meta-updater that bumps the aktualizr version (you can reuse mine; https://main.gitlab.in.here.com/olp/edge/ota/connect/client/meta-updater/tree/fix/ptest-c-api) and then run a pipeline on that branch with OE_PTEST set to 1. (You can also set OE_RPI to 0 to save some time.) |
https://main.gitlab.in.here.com/olp/edge/ota/connect/client/meta-updater/-/jobs/866481 - I suppose t_c_api_test is passed now (using "if(CMAKE_CROSSCOMPILING)"). |
Signed-off-by: Chekhovoi, Mykola <ext-mykola.chekhovoi@here.com>
5b2c3d0
to
d323579
Compare
No description provided.