New issue
Advanced search Search tips

Issue 687746 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

WPT importer does not delete -expected.txt for removed files

Project Member Reported by domenic@chromium.org, Feb 1 2017

Issue description

Checking out https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/wpt/streams/ today, I believe the file byte-length-queuing-strategy.serviceworker-expected.txt is leftover. There used to be a test named byte-length-queuing-strategy.serviceworker.html. However, it was renamed upstream to byte-length-queuing-strategy.serviceworker.https.html. The importer picked this up, but doesn't seem to have deleted the old -expected.txt file.

There are a lot of other leftovers in the streams directory and its subdirectories.
 
Components: -Infra Infra>Client>Chrome
Status: Available (was: Untriaged)
Cc: -qyears...@chromium.org
Components: -Infra>Client>Chrome Blink>Infra>Predictability
Owner: qyears...@chromium.org
Status: Assigned (was: Available)
There is a part of the import script which is *supposed* to remove these files:
https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py?l=239

So there's probably there somewhere.
Labels: Pri-2 Type-Bug
Taking another look at this. Notes:

The code that's supposed to remove the orphaned baselines looks like this:

  _log.info('Deleting any orphaned baselines.')
  is_baseline_filter = lambda fs, dirname, basename: self.is_baseline(basename)
  previous_baselines = self.fs.files_under(dest_path, file_filter=is_baseline_filter)
  for subpath in previous_baselines:
      full_path = self.fs.join(dest_path, subpath)
      if self.fs.glob(full_path.replace('-expected.txt', '*')) == [full_path]:
          self.fs.remove(full_path)

Now, suppose we rename the file byte-length-queuing-strategy.serviceworker.html so that it ends with .https.html. When the above code runs, I believe that:

 - previous_baselines would include 'byte-length-queuing-strategy.serviceworker-expected.txt'
 - self.fs.glob(full_path.replace('-expected.txt', '*'))
   would include both 'byte-length-queuing-strategy.serviceworker.https.html' and 'byte-length-queuing-strategy.serviceworker-expected.txt', so the baseline would not be deleted.

So, that's a bug.

The related baseline filename for a given test is splitext(test_filename)[0] + -expected.txt, which means that given a baseline, the expected filename can always be obtained by removing -expected.txt and adding a file extension with no "." characters.

https://chromium.googlesource.com/chromium/src/+/573bd7b3152aba99ff74ad02ae1aa846ad32a448/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py#531
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 7 2017

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

commit 10ac6df8184863142d3fa9b60b0349b3d062978a
Author: qyearsley <qyearsley@chromium.org>
Date: Tue Mar 07 01:23:58 2017

Fix orphaned wpt -expected.txt file deletion logic.

The previous logic was to use glob to check for corresponding
existing tests for a given baseline (-expected.txt) file, but
this is incorrect. The corresponding test for a given baseline
always involves adding one of a small set of possible extensions.

This CL also removes all files that would now be considered
orphaned baselines according to the new logic.

BUG= 687746 

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

[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/dom/ranges/Range-mutations-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/html/webappapis/scripting/events/messageevent-constructor-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/service-workers/cache-storage/serviceworker/credentials-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/byte-length-queuing-strategy.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/count-queuing-strategy.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/close-propagation-backward.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/close-propagation-forward.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/error-propagation-backward.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/error-propagation-forward.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/flow-control.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/general.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/multiple-propagation.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/pipe-through.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/piping/transform-streams.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-byte-streams/general.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/bad-strategies.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/bad-underlying-sources.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/brand-checks.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/cancel.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/count-queuing-strategy-integration.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/garbage-collection.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/general.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/pipe-through.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/readable-stream-reader.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/tee.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/readable-streams/templated.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/aborting.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/bad-strategies.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/bad-underlying-sinks.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/brand-checks.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/byte-length-queuing-strategy.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/close.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/constructor.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/count-queuing-strategy.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/general.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/start.serviceworker-expected.txt
[delete] https://crrev.com/a169810241444dc3eab4feabfcd272834c2622cd/third_party/WebKit/LayoutTests/external/wpt/streams/writable-streams/write.serviceworker-expected.txt
[modify] https://crrev.com/10ac6df8184863142d3fa9b60b0349b3d062978a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/base.py
[modify] https://crrev.com/10ac6df8184863142d3fa9b60b0349b3d062978a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py
[modify] https://crrev.com/10ac6df8184863142d3fa9b60b0349b3d062978a/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py

Status: Fixed (was: Assigned)
Components: Blink>Infra>Ecosystem
Components: -Blink>Infra>Predictability

Sign in to add a comment