Good catches!

There is one more "little" issue.  Because we skip to the end of /var/log/messages when the collector starts, we're always going to miss warnings that occur only between boot and collector start.  This is trickier to fix though.  Just a thought.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/bcecd2e227d1fe01c6be8e95bd8fd58f921cbfe3

commit bcecd2e227d1fe01c6be8e95bd8fd58f921cbfe3
Author: Mattias Nissler <mnissler@chromium.org>
Date: Fri Nov 25 11:01:55 2016

crash: Fix warn_collector tests.

This fixes test breakage introduced in 5aef999:
1. Fix warn_collector to not skip to the end after log rotation.
2. Fix the test to append the first warning to the log only after
   starting up warn_collector.

BUG= chromium:668666 
TEST=Manual runs of warn_collector_test.sh pass.

Change-Id: I48185d4d55b7b04fa397a8addb09f4ba1c24da59
Reviewed-on: https://chromium-review.googlesource.com/414088
Commit-Ready: Mattias Nissler <mnissler@chromium.org>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/bcecd2e227d1fe01c6be8e95bd8fd58f921cbfe3/crash-reporter/warn_collector.l
[modify] https://crrev.com/bcecd2e227d1fe01c6be8e95bd8fd58f921cbfe3/crash-reporter/warn_collector_test.sh

Re comment #1: As far as I understand, the current behavior of skipping to the end of the messages file is WAI though? So I'd not consider this a bug, but something we might want to improve. I share your doubts you expressed in the code comment though on complexity of doing this being worth it.

|===========================
|Field Name     ||Description
|`robot_id`     ||The ID of the robot that generated this comment.
|`robot_run_id` ||An ID of the run of the robot.
|`url`          |optional|URL to more information.
|`properties`   |optional|
Robot specific properties as map that maps arbitrary keys to values.
|`fix_suggestions`|optional|Suggested fixes for this robot comment as a list of
<<fix-suggestion-info,FixSuggestionInfo>> entities.
|===========================

#2 yes that was WAI, or let's just say it was a known limitation, it's not a regression.

The problem, of course, is that we don't know how far we've already parsed the file in previous runs of the service.  We could save a time stamp each time we send a warning, then skip to that point in the file.  We might not want to do it right at boot time, as it may impact boot speed (maybe not much, but everything counts).  There's going to be a race, but it doesn't matter.

It would be nice if we could automatically inspect logs from feedback reports and see how many "early warnings" we've been missing.

Thanks again for fixing this bug.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/3b2cbe875e6e49ee13bc8d38186620b75ec85052

commit 3b2cbe875e6e49ee13bc8d38186620b75ec85052
Author: Mattias Nissler <mnissler@chromium.org>
Date: Fri Nov 25 12:53:36 2016

crash: Prepare warn_collector test to run in sysroot.

This changes path handling in warn_collector_test.sh to allow it to
run from within the sysroot. Also, adjust crash-reporter.gyp to copy
the required resources to the sysroot.

BUG= chromium:668666 
TEST=./common-mk/platform2_test.py --board=$BOARD /var/cache/portage/chromeos-base/crash-reporter/out/Default/warn_collector_test.sh

Change-Id: I0116db9bd76eeec7826e3718df6d1cb9341a13ff
Reviewed-on: https://chromium-review.googlesource.com/414904
Commit-Ready: Mattias Nissler <mnissler@chromium.org>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/3b2cbe875e6e49ee13bc8d38186620b75ec85052/crash-reporter/crash-reporter.gyp
[modify] https://crrev.com/3b2cbe875e6e49ee13bc8d38186620b75ec85052/crash-reporter/warn_collector_test.sh

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/fd75a75d2330f9bbd2d8321890c062eae9df7856

commit fd75a75d2330f9bbd2d8321890c062eae9df7856
Author: Mattias Nissler <mnissler@chromium.org>
Date: Mon Apr 17 23:28:32 2017

crash-reporter: Run warn_collector test

This changes the crash-reporter ebuild to run the warn_collector test
as part of the test phase.

BUG= chromium:668666 
TEST=FEATURES=test emerge-$BOARD -v1 crash-reporter
CQ-DEPEND=CL:414904

Change-Id: I7ca3880c316d08fc3c514a8ddc3df71e7f25aeac
Reviewed-on: https://chromium-review.googlesource.com/414371
Commit-Ready: Mattias Nissler <mnissler@chromium.org>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/fd75a75d2330f9bbd2d8321890c062eae9df7856/chromeos-base/crash-reporter/crash-reporter-9999.ebuild