Feature/logfile

[9eCyberSec]

Added a flag to define a log file that saves the log seperatly.

[mimir-d]

at a glance, the test failure doesnt seem related, but just to make sure @9eCyberSec can you run ./run_tests.sh locally and see if it fails in the same way? You should just need docker (+compose) as deps

[9eCyberSec]

Locally it just works fine.

[mimir-d]

i've no idea why github didnt send any notifications for your last message, so sorry about that; i'll pull the branch and see why that test is failing

[mimir-d]

ok, so i ran this a couple of times over the last 2 weeks and it does fail sometimes with timeout. There have been some fixes for tests in the meantime, could you try to rebase on latest and push? Thanks

[mimir-d]

returning for the rebase

[llogen]

Thank you for your answer, i did the rebase. Hope it works now.

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (main@4ca8170). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main       #8   +/-   ##
=======================================
  Coverage        ?   66.14%           
=======================================
  Files           ?      156           
  Lines           ?     9347           
  Branches        ?        0           
=======================================
  Hits            ?     6183           
  Misses          ?     2498           
  Partials        ?      666           

Flag	Coverage Δ
e2e	50.39% <0.00%> (?)
integration	57.31% <0.00%> (?)
unittests	47.19% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out 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 4ca8170...a7f74b1. Read the comment docs.

[mimir-d]

cool, the tests run now; just need to signoff for the DCO

[mimir-d]

seems this didnt get any updates since last year; i'll comandeer later on and merge

The merge-base changed after approval.

[mimir-d]

@xaionaro does this make sense here? i dont know if we actually use logrus anymore

[xaionaro]

i dont know if we actually use logrus anymore

At least the upstream version of contest uses logrus:
https://github.com/linuxboot/contest/blob/develop/pkg/logging/default_options.go#L25

does this make sense here?

It kinda does. But a cleaner solution would be to feed an option to https://github.com/linuxboot/contest/blob/develop/pkg/logging/default_options.go#L21-L24 to add a multiwriter there if needed (to avoid type assertions; or at least to keep them local to where we make sure it is logrus). If one wants to avoid a type assertion at all then they can manually initialize a logrus instance and wrap it using this function: https://github.com/facebookincubator/go-belt/blob/main/tool/logger/implementation/logrus/logger.go#L484

Or if people are lazy, they can just move this line into the function WithBelt (to keep the assertion local to where we build a logrus logger).

[mimir-d]

yeah, that's what I was thinking. that type assertion looks kinda off to me. Not sure what we can do here, because this PR comes from a 9elements repo so we can't fix anything on this branch without the original author