Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Feat/c api updates #1387

Merged
merged 14 commits into from
Oct 29, 2019
Merged

Feat/c api updates #1387

merged 14 commits into from
Oct 29, 2019

Conversation

mchekhovoi
Copy link

OTA-3212 SendDeviceData()/SendManifest()/OpenStoredTarget() C APIs

@OYTIS
Copy link
Contributor

OYTIS commented Sep 24, 2019

This pull request introduces 1 alert when merging ba59e76 into 74fa9fd - view on LGTM.com

new alerts:

  • 1 for Return c_str of local std::string

Warning - Automated code review for advancedtelematic/aktualizr will be disabled on October 1, 2019. You can avoid this by installing the LGTM.com GitHub App. Read about the benefits of migrating to GitHub Apps in the blog.

Comment posted by LGTM.com

lbonn
lbonn reviewed Sep 25, 2019
View reviewed changes
Copy link
Contributor

@lbonn lbonn left a comment

Choose a reason for hiding this comment

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

There are also formatting and linting issues reported on CI.

For the formatting, just run make format to fix the issues automatically. For the lint, you'll have to look at the issues reported by make clang-tidy and fix them.

src/libaktualizr-c/libaktualizr-c.cc Outdated Show resolved Hide resolved
src/libaktualizr-c/libaktualizr-c.cc Outdated Show resolved Hide resolved
src/libaktualizr-c/test/api-test.c Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Sep 26, 2019 •
edited

Codecov Report

Merging #1387 into master will increase coverage by 0.08%.
The diff coverage is 76.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1387      +/-   ##
==========================================
+ Coverage   80.43%   80.51%   +0.08%     
==========================================
  Files         181      183       +2     
  Lines       10649    10911     +262     
==========================================
+ Hits         8565     8785     +220     
- Misses       2084     2126      +42
Impacted Files Coverage Δ
src/uptane_generator/repo.h 100% <ø> (ø) ⬆️
src/uptane_generator/uptane_repo.h 100% <ø> (ø) ⬆️
...baktualizr-c/test/api-test-utils/api-test-utils.cc 100% <100%> (ø)
src/uptane_generator/repo.cc 96.05% <100%> (+0.27%) ⬆️
src/libaktualizr/campaign/campaign.cc 87.01% <100%> (+4.25%) ⬆️
src/uptane_generator/uptane_repo.cc 100% <100%> (ø) ⬆️
src/uptane_generator/main.cc 82.75% <100%> (+0.3%) ⬆️
src/libaktualizr/campaign/campaign.h 57.14% <100%> (+7.14%) ⬆️
src/libaktualizr-c/libaktualizr-c.cc 52.67% <62.02%> (+52.67%) ⬆️
src/libaktualizr-c/test/api-test.c 69.64% <68.57%> (+69.64%) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c18ffa...777f51e. Read the comment docs.

Eugene Smirnov and others added 6 commits October 2, 2019 10:23
C API: initial implementation for Check()/Download()/Install()
7049faa 
Signed-off-by: Eugene Smirnov <evgenii.smirnov@here.com>
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
C API: initial implementation for SendDeviceData()/SendManifest()/Ope…
097c7a4 
…nStoredTarget()

Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
C API: initial implementation for SendDeviceData()/SendManifest()/Ope…
9ace4e8 
…nStoredTarget(), pt.2

Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
C API: api-test logging minor change
621dc59 
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
C API: initial implementation for using of fake_http_server and uptan…
904eda9 
…e_generator

Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
C API: test config generation was added for api-test
f620b58 
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
C API: implementation (pt.2) for Check()/Download()/Install()
b2e562c 
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
@mchekhovoi mchekhovoi force-pushed the feat/c-api-updates branch 3 times, most recently from 15b977f to 584b358 Compare October 24, 2019 09:10
OTA-3766 Consider using Ctest fixtures in C API unit tests
59db49d 
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
OTA-3210 SetSignalHandler() C API
0b44e3f 
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
Warning fixes for OTA-3210, OTA-3766, OTA-3765
aed7243 
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
pattivacek
pattivacek reviewed Oct 28, 2019
View reviewed changes
src/libaktualizr-c/test/api-test-utils/api-test-utils.cc Outdated
void Stop_fake_http_server(FakeHttpServer *server) {
// NOLINTNEXTLINE
if (server != nullptr && server->valid()) {
// NOLINTNEXTLINE
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add the linter ID here so that we know exactly what is being exempted?

OTA-3764 Extend uptane-generator tool and fake_test_server.py to supp…
065ca01 
…ort campaigns

Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
eu-siemann
eu-siemann previously approved these changes Oct 28, 2019
View reviewed changes
pattivacek
pattivacek reviewed Oct 28, 2019
View reviewed changes
Copy link
Collaborator

@pattivacek pattivacek left a comment •
edited

Choose a reason for hiding this comment

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

Just remembered we still have docs for uptane-generator at https://github.com/advancedtelematic/aktualizr/blob/master/docs/ota-client-guide/modules/ROOT/pages/uptane-generator.adoc. Probably worth adding an entry at the end.

Edit: wait, wrong PR... or did #1425 get merged into this PR?

@eu-siemann eu-siemann mentioned this pull request Oct 28, 2019
Closed
campaign: change fromJson() method to ctor
1e2a989 
Signed-off-by: Eugene Smirnov <evgenii.smirnov@here.com>
lbonn
lbonn reviewed Oct 28, 2019
View reviewed changes
src/libaktualizr/campaign/campaign_test.cc Outdated
TEST(campaign, Campaign_to_json) {
auto json = Utils::parseJSONFile(test_data_dir / "campaigns_sample.json");

auto campaigns = campaign::campaignsFromJson(json);
Copy link
Contributor

Choose a reason for hiding this comment

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

These two tests seem redundant. Furthermore, Campaign_to_json actually uses campaign::campaignsFromJson. We can probably just keep one of the two.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed. Only "Campaigns_to_json" test is remained.

Mykola Chekhovoi added 2 commits October 28, 2019 18:19
Redundant warning suppression removed from api-test-utils.cc
314c0f0 
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
OTA-3764, campaign: UT changes after code-review
777f51e 
Signed-off-by: Mykola Chekhovoi <ext-mykola.chekhovoi@here.com>
@eu-siemann eu-siemann merged commit 0fcb778 into master Oct 29, 2019
@eu-siemann eu-siemann deleted the feat/c-api-updates branch October 29, 2019 10:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lbonn lbonn lbonn left review comments

@pattivacek pattivacek pattivacek left review comments

@eu-siemann eu-siemann eu-siemann approved these changes

@kbushgit kbushgit Awaiting requested review from kbushgit

@mike-sul mike-sul Awaiting requested review from mike-sul

@xcheng-here xcheng-here Awaiting requested review from xcheng-here

@Zee314159 Zee314159 Awaiting requested review from Zee314159

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

Successfully merging this pull request may close these issues.

None yet
6 participants
@mchekhovoi @OYTIS @codecov-io @lbonn @pattivacek @eu-siemann