New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 819478 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Audit existing Files.app tests

Project Member Reported by sashab@chromium.org, Mar 7 2018

Issue description

Audit the current Files.app tests we have (and which are enabled) to help us get a better understanding of our testing coverage, and where we need to improve.

Ideally, also produce a doc on how to add new tests, mainly for UI integration tests (such as touch bug tests) as well as how to run/watch/debug them.
 
Labels: M-67
Status: Started (was: Assigned)
First glance shows that we have:
* unit tests:
 - ui/file_manager/**/*_unittest.[js|html]
 - Run in chrome/browser/chromeos/file_manager/file_manager_jstest.cc

* integration tests:
 - ui/file_manager/integration_tests/*
 - included in ui/file_manager/integration_tests/file_manager_test_manifest.json
 - Run in chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

* Run tests (currently 45):
out/Default/browser_tests --gtest_filter=FileManager*
40 Unit tests run with --gtest_filter=FileManagerJSTest.*
102 Integration tests run with --gtest_filter=*/FileManagerBrowserTest.*

Summary of tests:
https://docs.google.com/spreadsheets/d/1iuDr0thscc5LPM2UJUfaaSAOPXL7MceFKsIepg8uiDc/edit#gid=0
Document added with disabled state of integration tests.
Unit Tests: see #c3
Integration Tests: see #c3
API tests:
 - c++ unit tests for chrome APIs such as chrome.fileManagerPrivate.
 - https://cs.chromium.org/chromium/src/chrome/browser/chromeos/extensions/file_manager/file_manager_private_apitest.cc
 - runs as InProcBrowserTest

Comment 7 by sashab@chromium.org, Mar 22 2018

Cc: lucmult@chromium.org
I see the remaining work here is to better understand the integration tests.

1/ What is causing the flakes?
2/ What amount of the backend is using fakes/mocks, and how much of actual backend code is running?

Comment 9 by sashab@chromium.org, Mar 28 2018

Labels: -M-67 M-68
Owner: noel@chromium.org
Cc: joelhockey@chromium.org
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 17 2018

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

commit df79950240771a48655a06a1037c64a96c01d5a4
Author: Joel Hockey <joelhockey@chromium.org>
Date: Tue Apr 17 03:12:44 2018

Pause FileManagerUITests.

Turn off FileManagerUITests from CQ until current audit
of tests by noel@ is complete.

Bug:  819478 
Change-Id: I14dca567de4fb7919d94b1d89a0cb2dada5b8d67
Reviewed-on: https://chromium-review.googlesource.com/1013668
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551233}
[modify] https://crrev.com/df79950240771a48655a06a1037c64a96c01d5a4/chrome/browser/chromeos/file_manager/file_manager_uitest.cc

Labels: -Pri-1 Pri-2
This work is going great - lowering priority to P2.
Labels: -M-68 M-69
1/ What is causing the flakes?

answ: Starting tests before the test resources were set up.

2/ What amount of the backend is using fakes/mocks, and how much of actual backend code is running?

answ: In chrome, unit tests mock the needed FileApps/FSP features, integration tests mock the needed Volumes (Drive, MTP, USB whatever). There is no need to
bring up "actual backend" code [1].

[1] back-end code systems should provide/maintain the mock, and it should come with unit tests (see ToTT #528).
The flake cause fixed on  crbug.com/831074#c15 , and all tests re-enabled.

Feel I inherited this bug somehow due to that, but I have no clue what "Audit" means or actual intent or work needed on this bug.

I can list || count them and compare to #

FileManagerPrivateApiTest "unittests" 5 => 6 tests

for f in *FileManagerPrivateApiTest*; do
  ./out/Release/browser_tests --gtest_filter="$f" --gtest_list_tests | \
     sed "s|  #.*$||g"
done

FileApp JS "unittests" 40 => 42 tests

for f in *FileManagerJs*; do
  ./out/Release/browser_tests --gtest_filter="$f" --gtest_list_tests | \
     sed "s|  #.*$||g"
done

FileApp "integration tests" 102 => 322 tests

for f in *FilesAppBrowser* \
   *AudioPlayerBrowserTest* \
   *GalleryBrowserTest* \
   *VideoPlayerBrowserTest* \
   *MultiProfileFileManagerBrowserTest* ; do
  ./out/Release/browser_tests --gtest_filter="$f" --gtest_list_tests | \
     sed "s|  #.*$||g"
done
Cc: -joelhockey@chromium.org noel@chromium.org
Owner: joelhockey@chromium.org
> FileApp "integration tests" 102 => 322 tests

Seems folks know how to write integration tests, and have added a few.

Still dunno what "Audit" means in this bug, re-assigning back to the owner.
Status: Fixed (was: Started)
No worries, I think this bug is complete given that we all know how to write tests (and they are documented at go/xf-noogler). We also have a much better idea of coverage.
Marking as fixed.
For "coverage", as in real coverage reports, filed issue 865376.

Sign in to add a comment