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]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.14%. Comparing base (e0b9152) to head (a7f74b1).
⚠️ Report is 184 commits behind head on develop.

Files with missing lines	Patch %	Lines
cmds/contest/server/server.go	14.28%	5 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (14.28%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop       #8      +/-   ##
===========================================
- Coverage    66.23%   66.14%   -0.09%     
===========================================
  Files          156      156              
  Lines         9340     9347       +7     
===========================================
- Hits          6186     6183       -3     
- Misses        2489     2498       +9     
- Partials       665      666       +1     

Flag	Coverage Δ
e2e	50.39% <14.28%> (-0.03%)	⬇️
integration	57.31% <ø> (-0.04%)	⬇️
unittests	47.19% <ø> (-0.09%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[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