New issue
Advanced search Search tips

Issue 612296 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Some tests don't cleanup a temporary file that they create before completing.

Project Member Reported by vakh@chromium.org, May 16 2016

Issue description

DownloadProtectionServiceTest.CheckClientDownloadZip and DownloadProtectionServiceTest.CheckClientDownloadReportCorruptZip don't delete the temporary file "a.tmp" that they create.

This file gets created in the current working directory.
 

Comment 1 by vakh@chromium.org, May 16 2016

Owner: vakh@chromium.org
Status: Started (was: Available)

Comment 2 by vakh@chromium.org, May 16 2016

The tests that exhibit this are, all in DownloadProtectionServiceTest:
1. CheckClientDownloadZip
2. CheckClientDownloadReportCorruptZipNormal
3. CheckClientDownloadReportCorruptZipExtended
4. CheckClientDownloadReportCorruptZipIncognito
Project Member

Comment 3 by bugdroid1@chromium.org, May 17 2016

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

commit e4f9ac949175541f148eee4b11488c30b7177623
Author: vakh <vakh@chromium.org>
Date: Tue May 17 06:32:01 2016

Delete tmp_path_ at the end of the tests

BUG= 612296 

Review-Url: https://codereview.chromium.org/1984763003
Cr-Commit-Position: refs/heads/master@{#394069}

[modify] https://crrev.com/e4f9ac949175541f148eee4b11488c30b7177623/chrome/browser/safe_browsing/download_protection_service_unittest.cc

Comment 4 by vakh@chromium.org, May 17 2016

Status: Verified (was: Started)

Comment 5 by vakh@chromium.org, May 17 2016

Cc: asanka@chromium.org nparker@chromium.org
Status: Started (was: Verified)
[re-opening]
asanka@ pointed out that it's better to use a base::ScopedTempDir for the following reasons:
* the resulting path is entirely under the control of the test, and is known to
be writeable by the test binary,
* the resulting path is unique (i.e. the tests won't fail if there are
concurrent tests running)
* even if the test fails or crashes the files will still be cleaned up by the
platform at some point.
Project Member

Comment 6 by bugdroid1@chromium.org, May 22 2016

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

commit a1d3a937bb232bef874e846f4f5378f23554e27e
Author: vakh <vakh@chromium.org>
Date: Sun May 22 23:41:12 2016

Use a temp directory for the temporary files that the tests create.

BUG= 612296 

Review-Url: https://codereview.chromium.org/1983153003
Cr-Commit-Position: refs/heads/master@{#395277}

[modify] https://crrev.com/a1d3a937bb232bef874e846f4f5378f23554e27e/chrome/browser/safe_browsing/download_protection_service_unittest.cc

Comment 7 by vakh@chromium.org, May 23 2016

Status: Fixed (was: Started)

Sign in to add a comment