Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Only consider tests to be manual tests as a last resort.#199

Closed
qyearsley-zz wants to merge 1 commit into
w3c:masterfrom
qyearsley-zz:change-manifest
Closed

Only consider tests to be manual tests as a last resort.#199
qyearsley-zz wants to merge 1 commit into
w3c:masterfrom
qyearsley-zz:change-manifest

Conversation

@qyearsley-zz

@qyearsley-zz qyearsley-zz commented Apr 4, 2017

Copy link
Copy Markdown

@gsnedders @jgraham

This is one very simple idea for #196 - would this be OK to do? This would result in many tests being reclassified from "manual" to "testharness" or "reftest".

Context: Previously for Chromium we had some logic that would look at the test file itself and look for "testharness.js" or "" to guess if the test was a testharness or reftest, and not import it if didn't look like either of those, and then run it as a testharness or ref test regardless of what the manifest says. But, it would be better to rely on what the manifest says, so I want to remove that logic.


This change is Reviewable

@jgraham

jgraham commented Apr 4, 2017

Copy link
Copy Markdown
Member

@gsnedders already complained about my much more conservative change in #197 which is believed by inspection not to break anything. So I'm going to say that this change which does (we certainly have manual tests that also use testharness.js) isn't a great idea.

@jgraham jgraham closed this Apr 4, 2017
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #199 into master will not change coverage.
The diff coverage is 50%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #199   +/-   ##
=======================================
  Coverage   87.75%   87.75%           
=======================================
  Files          24       24           
  Lines        2418     2418           
  Branches      406      406           
=======================================
  Hits         2122     2122           
  Misses        230      230           
  Partials       66       66
Impacted Files Coverage Δ
manifest/sourcefile.py 82.54% <50%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51ba63c...6ab7cc8. Read the comment docs.

@qyearsley-zz qyearsley-zz deleted the change-manifest branch April 6, 2017 00:28
@qyearsley-zz

Copy link
Copy Markdown
Author

Hm, alright :-) thanks for quickly taking a look and making a decision.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants