New issue
Advanced search Search tips

Issue 801192 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

check_network_annotations seems to fail all/many of the try runs "with patch" on linux_chromium_rel_ng

Project Member Reported by brat...@opera.com, Jan 11 2018

Issue description

Status: Started (was: Untriaged)
I am disabling the test for more study.
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17eb88b786ae61246524887c184eb73986f97cf2

commit 17eb88b786ae61246524887c184eb73986f97cf2
Author: rhalavati@google.com <rhalavati@google.com>
Date: Thu Jan 11 17:39:52 2018

Disable network traffic annotation Test.

Network traffic annotation test script fails on some runs.
Disabling it for more study.

TBR=msramek

Bug:  801192 
Change-Id: Id275d4e7d034b4865d52d3b23d66391d3a9dbda7
Reviewed-on: https://chromium-review.googlesource.com/860413
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528660}
[modify] https://crrev.com/17eb88b786ae61246524887c184eb73986f97cf2/tools/traffic_annotation/scripts/check_annotations.py

Cc: msramek@chromium.org
The auditor returned an error when none of the changed files had any relation to traffic annotation, and all of my former tests had somehow some relation to it.

I updated the auditor and checking script in this CL (crrev.com/c/863483) with the following two changes:
 1- If changed files are not relevant, no error is returned.
 2- Auditor is called with a '--nice' switch, which tells it to return
    error, ONLY, if all checks are correctly performed and some problems in
    annotations are found. Using this flag, we won't have any spamming if
    something unexpected does not work and we can have a local/waterfall
    version running to ensure that the trybot has not missed anything.

I have also the following 5 test CLs that are run before activating the trybots:
 - crrev.com/c/859279:
   Runs the auditor on the whole repository to ensure everything is consistent.
 - crrev.com/c/863484:
   Runs the auditor on a set of irrelevant files, ensuring there is no
   errors.
 - crrev.com/c/859278:
   Runs the auditor on a set of irrelevant changes in relevant files,
   ensuring there is no errors.
 - crrev.com/c/859337:
   Runs the auditor on a set of files with annotation errors, to check if 
   there are errors.
 - crrev.com/c/859338:
   Runs the auditor on some changed annotation that requires annotations.xml
   update, to check if there is an error.

It seems to me that this time all cases are covered, and there is safegaurd in place for unforeseen cases.

Do you have any suggestions? Can I make the tests more automatic for future updates?

Project Member

Comment 6 by bugdroid1@chromium.org, Jan 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3efb8443e8e9475ab0fa600fb594d0aa6168a6c5

commit 3efb8443e8e9475ab0fa600fb594d0aa6168a6c5
Author: rhalavati@google.com <rhalavati@google.com>
Date: Fri Jan 12 16:14:36 2018

Make network traffic annotation auditor error resilient.

A switch is added to network traffic annotation auditor to avoid
returning error, if there would be an operational error preventing it
from actual testing. This '--error-resilient' switch will be used on
the trybot to avoid spamming when the test cannot be run.

Auditor is also updated to return no error (even without the new
switch) if the given path filters do not include any relevant file.

Bug:  690323 
Bug:  801192 
Change-Id: I4aa3172c356d6daa9403c8a0b5447cefe2141f5c
Reviewed-on: https://chromium-review.googlesource.com/863483
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528959}
[modify] https://crrev.com/3efb8443e8e9475ab0fa600fb594d0aa6168a6c5/tools/traffic_annotation/README.md
[modify] https://crrev.com/3efb8443e8e9475ab0fa600fb594d0aa6168a6c5/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc
[modify] https://crrev.com/3efb8443e8e9475ab0fa600fb594d0aa6168a6c5/tools/traffic_annotation/auditor/traffic_annotation_auditor.h
[modify] https://crrev.com/3efb8443e8e9475ab0fa600fb594d0aa6168a6c5/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc
[modify] https://crrev.com/3efb8443e8e9475ab0fa600fb594d0aa6168a6c5/tools/traffic_annotation/bin/linux64/traffic_annotation_auditor.sha1
[modify] https://crrev.com/3efb8443e8e9475ab0fa600fb594d0aa6168a6c5/tools/traffic_annotation/scripts/check_annotations.py

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/64c23fad92332d3b445989bbc535ac9d498311b1

commit 64c23fad92332d3b445989bbc535ac9d498311b1
Author: rhalavati@google.com <rhalavati@google.com>
Date: Mon Jan 15 09:07:43 2018

Activate network traffic annotation trybot.

After updating the traffic annotation and check script in
crrev.com/c/863483, network traffic annotation trybot is activated.

Bug:  801192 
Bug:  788035 
Change-Id: I3bdc2bab33445cb8042c773ec5cca4017e8b3027
Reviewed-on: https://chromium-review.googlesource.com/863482
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529232}
[modify] https://crrev.com/64c23fad92332d3b445989bbc535ac9d498311b1/tools/traffic_annotation/scripts/check_annotations.py

Status: Fixed (was: Started)
It's been running for a day without errors.

Sign in to add a comment