New issue
Advanced search Search tips

Issue 825659 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Add readme to ui/file_manager/file_manager/test

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

Issue description

Remove the testing page & associated browsertests.

This was meant as a prototype, but we haven't agreed that it's a good way forward. New tests are being added and there are problems; e.g. files generated by the testing script are added to local patches (no .gitignore), and patches are being blocked because of the tests failing on the bots due to missing chrome.metricsPrivate mocks.

Since I don't think we are going to commit to maintaining this, let's roll it back for now until we have a proper testing solution. We can refer to the original commits when we want to return and try again.

Sasha
 
I'm reluctant to delete this.  At the least, it is very useful as a dev tool for me.  It is also ideal for testing UMA, and the existing delete.js test has already proven useful to add to our test coverage.

There is a .gitignore file which should be stopping any accidental checkins.
https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/.gitignore

Can you point me at the failing tests?  The test.html page has always had an implementation of chrome.metricsPrivate in https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/test/js/chrome_api_test_impl.js?l=71&rcl=95767e0e0acab6579466b07cd8003185cc892253

 

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

Summary: Add readme to ui/file_manager/file_manager/test (was: Remove ui/file_manager/file_manager/test)
Since this has been a useful development tool, may I propose we add a README to this folder instead.

Something along the lines of:

- This test system is in alpha
- Please contact joelhockey@ or sashab@ before adding a dependency on this to see if its actually what you need
- It could go away at any moment
- Etc

Just to make sure nobody adds a dependency on us, or thinks this is our long-term plan because the code is checked in :)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2018

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

commit 6cd05b83acb325af019d4cac627217e38f882742
Author: Joel Hockey <joelhockey@chromium.org>
Date: Thu Mar 29 02:43:34 2018

Add readme to ui/file_manager/file_manager/test

Bug:  825659 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I6c4ba182597aa58f8f8d1a12d1112b4bce75e9aa
Reviewed-on: https://chromium-review.googlesource.com/984753
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546709}
[add] https://crrev.com/6cd05b83acb325af019d4cac627217e38f882742/ui/file_manager/file_manager/test/README

Status: Fixed (was: Assigned)

Sign in to add a comment