Network Traffic annotation tests do not block moving/adding annotations to test code. |
|||
Issue descriptionIf a network traffic annotation is added/moved to a test file and is not part of Chrome binary on any platform, it would still need to be in the tools/traffic_annotation/summary/annotations.xml file, marked as deprecated, and its file path is kept. e.g., see this line in the annotations.xml file: <item id="expect_ct_reporter" hash_code="57276415" type="0" deprecated="2018-06-20" content_hash_code="130492494" file_path="services/network/expect_ct_reporter.cc"/> Expected behavior: If an annotation is not used on any platform for none-test code, traffic_annotation_auditor tests should fail and request replacement of the annotation with a TRAFFIC_ANNOTATION_FOR_TESTS tag.
,
Oct 31
+nicolaso@, Just thought this can be the next step after the other two fixes.
,
Oct 31
Assigning to myself, but it may take some time before I take a stab at it.
,
Nov 14
Trying to makes sure I understand the problem space here. Here's how I understand it, but correct me if I'm wrong. There are 2 'kinds' of .cc/.h/.mm files: (a) Files that are part of the chrome binary (dependencies of 'chrome' in GN). (b) Any other files (e.g. test files) Files in (a) can declare and use annotations in their source code. Files in (b) should not declare annotations. Now, I'm not sure I understand how the example you gave is relevant. It looks like the 'expect_ct_reporter' annotation is declared in 'expect_ct_reporter.cc', which is *not* a test file(?). It also looks like it's being used in the source code [1], so I don't see why it's marked as deprecated in the first place. I'm probably missing something. [1] https://cs.chromium.org/chromium/src/services/network/expect_ct_reporter.cc?l=254&rcl=b3c1d9dd2fc2c97126a239d6c321a13a2d978202
,
Nov 15
Yes, it seems that the use case is changed since I filed the bug. It's clearly a valid use now. In relation to https://crbug.com/904770 , maybe the best approach would be to totally ignore test files and if there would be any annotation that is not part of non-test files, we would ignore it and not add it to any report. |
|||
►
Sign in to add a comment |
|||
Comment 1 by isandrk@chromium.org
, Jun 21 2018