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

Prefer automated tests for css tests with interact or font flags#197

Open
jgraham wants to merge 1 commit into
masterfrom
css_manifest_order
Open

Prefer automated tests for css tests with interact or font flags#197
jgraham wants to merge 1 commit into
masterfrom
css_manifest_order

Conversation

@jgraham

@jgraham jgraham commented Apr 4, 2017

Copy link
Copy Markdown
Member

For tests with "interact" or "font" flags that are also marked as
reftests or testharness.js tests, prefer running the test
automatically vs assuming it's a manual test. For "interact" flags it
has been verified that this is reasonable. For the "font" flag this is
likely to produce some false positives (things detected as tests that
are not actually able to run correctly without installing some other
font), but the status quo is to not run many tests that use
@font-face, so this is an improvement, even though it's not quite right.


This change is Reviewable

@jgraham

jgraham commented Apr 4, 2017

Copy link
Copy Markdown
Member Author

Fixes #196

@jgraham jgraham force-pushed the css_manifest_order branch from ec23c64 to 7884be0 Compare April 4, 2017 11:07
@annevk

annevk commented Apr 4, 2017

Copy link
Copy Markdown
Member

I actually think we should change the tests instead to remove the "interact" flag since they're not interactive at all.

For tests with "interact" flag that are also marked as reftests or
testharness.js tests, prefer running the test automatically vs
assuming it's a manual test. It has been verified that this is
reasonable based on an insepction of the existing tests.
@jgraham jgraham force-pushed the css_manifest_order branch from 7884be0 to d673bdc Compare April 4, 2017 12:18
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #197 into master will increase coverage by 1.62%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
+ Coverage   87.75%   89.38%   +1.62%     
==========================================
  Files          24       24              
  Lines        2418     2722     +304     
  Branches      406      505      +99     
==========================================
+ Hits         2122     2433     +311     
+ Misses        230      223       -7     
  Partials       66       66
Impacted Files Coverage Δ
manifest/tests/test_sourcefile.py 100% <100%> (ø) ⬆️
manifest/sourcefile.py 84.69% <100%> (+2.15%) ⬆️
lint/fnmatch.py 71.42% <0%> (ø) ⬆️
lint/tests/test_lint.py 100% <0%> (ø) ⬆️
lint/lint.py 92.85% <0%> (+3.05%) ⬆️

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 b26b982...d673bdc. Read the comment docs.

@jgraham

jgraham commented Apr 4, 2017

Copy link
Copy Markdown
Member Author

@annevk Sure, happy to accept a patch to do that. I still think this is a reasonable change to take.

@gsnedders

Copy link
Copy Markdown
Member

Once web-platform-tests/wpt#5341 lands, this will misrun 829 tests that require fonts to be installed.

@gsnedders

Copy link
Copy Markdown
Member

I would strongly, strongly prefer just fixing the metadata.

mrego added a commit to mrego/wpt that referenced this pull request Apr 4, 2017
There were a bunch of tests that have both a reference file and
the "interact" flag.
They use some small JavaScript to execute some steps
before checking the output against reference file.
This caused them to be wrongly considered manual tests
when they can be run automatically.

For more info check w3c/wpt-tools#196 and w3c/wpt-tools#197.
mrego added a commit to mrego/wpt that referenced this pull request Apr 5, 2017
There were a bunch of tests that have both a reference file and
the "interact" flag.
They use some small JavaScript to execute some steps
before checking the output against reference file.
This caused them to be wrongly considered manual tests
when they can be run automatically.

For more info check w3c/wpt-tools#196 and w3c/wpt-tools#197.
mrego added a commit to mrego/wpt that referenced this pull request Apr 7, 2017
There were a bunch of tests that have both a reference file and
the "interact" flag.
They use some small JavaScript to execute some steps
before checking the output against reference file.
This caused them to be wrongly considered manual tests
when they can be run automatically.

For more info check w3c/wpt-tools#196 and w3c/wpt-tools#197.
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.

4 participants