Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate metadata_root and expectation_root #7975

Conversation

lukebjerring
Copy link
Contributor

@lukebjerring lukebjerring commented Oct 25, 2017

WIP for #7959


This change is Reviewable

@ghost
Copy link

ghost commented Oct 25, 2017

Build PASSED

Started: 2017-10-26 16:20:41
Finished: 2017-10-26 16:35:31

View more information about this build on:

@jgraham
Copy link
Contributor

jgraham commented Oct 26, 2017

Reviewed 3 of 7 files at r1.
Review status: 3 of 7 files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.


tools/ci/check_stability.py, line 167 at r1 (raw file):

                        action="store",
                        default=wpt_root,
                        help="Directory that will contain MANIFEST.json")

I wonder if we should change so that there's an explicit argument that's the path to the manifest file, rather than a directory.


tools/ci/check_stability.py, line 168 at r1 (raw file):

                        default=wpt_root,
                        help="Directory that will contain MANIFEST.json")
    parser.add_argument("--expectation_root",

Should be expectation-root.


tools/ci/check_stability.py, line 168 at r1 (raw file):

                        default=wpt_root,
                        help="Directory that will contain MANIFEST.json")
    parser.add_argument("--expectation_root",

Need to add all these arguments to tools/wptrunner/wptrunner/wptcommandline.py in several places.


tools/wpt/run.py, line 66 at r1 (raw file):

    kwargs.set_if_none("tests_root", wpt_root)
    kwargs.set_if_none("metadata_root", wpt_root)
    kwargs.set_if_none("expectation_root", kwargs.get("metadata_root"))

I think this should go in wptcommandline.py in wptrunner, so it applies to all users of wptrunner, not just people going through the wpt frontend.


Comments from Reviewable

Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a reasonable start, but there are still a bunch of places where the metadata path is used for expectation metadata e.g in testloader.py.

@gsnedders
Copy link
Member

this was done in #9676

@gsnedders gsnedders closed this Feb 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants