New issue
Advanced search Search tips

Issue 894334 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Java tests with 2+ annotations are broken by format/presubmit

Project Member Reported by s...@chromium.org, Oct 11

Issue description

Trying to update FeedRefreshTaskTest.java, I ran into a problem. I wanted to add an annotation such that the file would look like

@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add(ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE)
@Features.EnableFeatures(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
public class FeedRefreshTaskTest {

However, after "git cl format" (which was forced by PRESUMIT), it looked like

@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags
        .Add(ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE)
        @Features.EnableFeatures(ChromeFeatureList.INTEREST_FEED_CONTENT_SUGGESTIONS)
        public class FeedRefreshTaskTest {

While this looks bad, it's actually much worse. When we compile the class, we get some sort of forced lint error!

  File "../../build/android/gyp/javac.py", line 622, in <module>
    sys.exit(main(sys.argv[1:]))
  File "../../build/android/gyp/javac.py", line 618, in main
    add_pydeps=False)
  File "/usr/local/google/home/skym/chromium/clankium/src/build/android/gyp/util/build_utils.py", line 597, in CallAndWriteDepfileIfStale
    pass_changes=True)
  File "/usr/local/google/home/skym/chromium/clankium/src/build/android/gyp/util/md5_check.py", line 87, in CallAndRecordIfStale
    function(*args)
  File "/usr/local/google/home/skym/chromium/clankium/src/build/android/gyp/util/build_utils.py", line 582, in on_stale_md5
    function(*args)
  File "../../build/android/gyp/javac.py", line 610, in <lambda>
    classpath_inputs, classpath),
  File "../../build/android/gyp/javac.py", line 388, in _OnStaleMd5
    _CreateInfoFile(java_files, options, srcjar_files, javac_generated_sources)
  File "../../build/android/gyp/javac.py", line 249, in _CreateInfoFile
    'Chromium java files must only have one class: {}'.format(source))
AssertionError: Chromium java files must only have one class: ../../chrome/android/javatests/src/org/chromium/chrome/browser/feed/FeedRefreshTaskTest.java
ninja: build stopped: subcommand failed.

Essentially stopping anyone from having a java test class with 2+ annotations. If we remove either of the two annotations, then the weird formatting goes away.
 
This sounds more like an issue with whatever git cl format is using to format java code?
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 11

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

commit cc91158a5505d55b66cd01b5da2d4ad973e9ffd9
Author: Justin DeWitt <dewittj@chromium.org>
Date: Tue Dec 11 15:25:48 2018

[EoS] Fix formatting of ExploreSitesPageTest

This causes a lint error on the official builders.

Bug: 894334,913755, 913012 
Change-Id: I9dbaa9932873755639d22618b3d758ef87c840cb
Reviewed-on: https://chromium-review.googlesource.com/c/1370587
Reviewed-by: Cathy Li <chili@chromium.org>
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615526}
[modify] https://crrev.com/cc91158a5505d55b66cd01b5da2d4ad973e9ffd9/chrome/android/javatests/src/org/chromium/chrome/browser/explore_sites/ExploreSitesPageTest.java

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 11

Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/927766a8531e6c7f08fccf93bc0903d004a08985

commit 927766a8531e6c7f08fccf93bc0903d004a08985
Author: Justin DeWitt <dewittj@chromium.org>
Date: Tue Dec 11 22:05:03 2018

[EoS] Fix formatting of ExploreSitesPageTest

This causes a lint error on the official builders.

Bug: 894334,913755, 913012 
Change-Id: I9dbaa9932873755639d22618b3d758ef87c840cb
Reviewed-on: https://chromium-review.googlesource.com/c/1370587
Reviewed-by: Cathy Li <chili@chromium.org>
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615526}(cherry picked from commit cc91158a5505d55b66cd01b5da2d4ad973e9ffd9)
Reviewed-on: https://chromium-review.googlesource.com/c/1372447
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#264}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/927766a8531e6c7f08fccf93bc0903d004a08985/chrome/android/javatests/src/org/chromium/chrome/browser/explore_sites/ExploreSitesPageTest.java

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/927766a8531e6c7f08fccf93bc0903d004a08985

Commit: 927766a8531e6c7f08fccf93bc0903d004a08985
Author: dewittj@chromium.org
Commiter: dewittj@chromium.org
Date: 2018-12-11 22:05:03 +0000 UTC

[EoS] Fix formatting of ExploreSitesPageTest

This causes a lint error on the official builders.

Bug: 894334,913755, 913012 
Change-Id: I9dbaa9932873755639d22618b3d758ef87c840cb
Reviewed-on: https://chromium-review.googlesource.com/c/1370587
Reviewed-by: Cathy Li <chili@chromium.org>
Commit-Queue: Justin DeWitt <dewittj@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615526}(cherry picked from commit cc91158a5505d55b66cd01b5da2d4ad973e9ffd9)
Reviewed-on: https://chromium-review.googlesource.com/c/1372447
Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#264}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment