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

Get rid of running modes. #1039

Merged
merged 4 commits into from
Dec 18, 2018
Merged

Get rid of running modes. #1039

merged 4 commits into from
Dec 18, 2018

Conversation

pattivacek
Copy link
Collaborator

aktualizr still has an input parameter (--run-mode) that does the same thing, but this is no longer wired directly into libaktualizr; it exists purely in the wrapper outside it. This should be much cleaner and simpler to use.

Note that once upon a time I advocated for all aktualizr commandline options to be also represented in the config. I no longer believe this to be a good idea for things that only matter outside of libaktualizr. An internal "run mode" for libaktualizr does not make sense. It should be driven by the code outside of the API.

aktualizr still has an input parameter (--run-mode) that does the same
thing, but this is no longer wired directly into libaktualizr; it exists
purely in the wrapper outside it. This should be much cleaner and
simpler to use.

Note that once upon a time I advocated for all aktualizr commandline
options to be also represented in the config. I no longer believe this
to be a good idea for things that only matter outside of libaktualizr.
An internal "run mode" for libaktualizr does not make sense. It should
be driven by the code outside of the API.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
If we have to use FAIL(), we should at least provide an explanation to
the user running the test.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
Testing that nothing is downloaded without calling Download() explicitly
is already covered by the other download tests. Testing that UptaneCycle
does what we expect when no updates are found is perhaps slightly more
interesting, however.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
@lbonn
Copy link
Contributor

lbonn commented Dec 18, 2018

Great, thanks! Some instances left (bad doc and failing tests):

$ git grep running-mode
docs/selectively-triggering-aktualizr.adoc:It is also possible to trigger the update cycle, or individual parts of the update cycle, manually or programmatically. This is done by using aktualizr's `--running-mode` option, or just supplying the operation name as a subcommand.
docs/selectively-triggering-aktualizr.adoc:Installation reports are sent when aktualizr polls the server for updates, so the `check` running-mode should be used again after installing:
scripts/test_aktualizr_repo.sh:aktualizr --config "${TMPDIR}/sota.toml" --running-mode once
scripts/test_install_aktualizr_and_update.sh:aktualizr -c /persistent/selfupdate.toml -c "$TEMP_DIR/conf.toml" --running-mode=once

Sorry, "running-mode" was always slightly weird to me.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
@pattivacek
Copy link
Collaborator Author

Some instances left (bad doc and failing tests)

On it, I noticed at the same time you did!

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.

Finally :). Ok when CI is green

@codecov-io
Copy link

codecov-io commented Dec 18, 2018

Codecov Report

Merging #1039 into master will increase coverage by 0.03%.
The diff coverage is 61.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1039      +/-   ##
==========================================
+ Coverage   82.21%   82.25%   +0.03%     
==========================================
  Files         196      196              
  Lines       13825    13787      -38     
==========================================
- Hits        11366    11340      -26     
+ Misses       2459     2447      -12
Impacted Files Coverage Δ
src/libaktualizr/primary/aktualizr.h 100% <ø> (ø) ⬆️
src/libaktualizr/config/config.h 100% <ø> (ø) ⬆️
src/libaktualizr/config/config.cc 95.65% <ø> (+0.39%) ⬆️
src/libaktualizr/utilities/types.cc 70.68% <ø> (+9.15%) ⬆️
src/libaktualizr/uptane/uptane_test.cc 98.03% <ø> (-0.01%) ⬇️
src/libaktualizr/primary/sotauptaneclient.h 100% <ø> (ø) ⬆️
src/libaktualizr/utilities/config_utils.h 92.75% <ø> (-0.4%) ⬇️
src/libaktualizr/utilities/types.h 76.92% <ø> (ø) ⬆️
src/libaktualizr/config/config_test.cc 98.88% <100%> (-0.02%) ⬇️
src/libaktualizr/primary/aktualizr.cc 71.96% <100%> (-2.15%) ⬇️
... and 3 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 b1c3e87...07c5809. Read the comment docs.

@lbonn lbonn merged commit c7d0c47 into master Dec 18, 2018
@lbonn lbonn deleted the fix/rm-running-mode branch December 18, 2018 16:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants