Github action for custom metric tests with WPT API

[max-ostapenko]

Resolves #6

As pointed in README, each custom metric adjustment can be tested with WPT.

PR includes:

• test workflow,
• WPT runner script,
• unit tests,
• parsing custom websites to test from PR body (see PR template).

---

Test websites:

• https://example.com/

[rviscomi]

Oh wow thank you for picking this up!

[max-ostapenko]

Unfortunately WebPageTest/webpagetest-api doesn't support POST requests to WPT API.
So when we try to attach custom metrics code (privacy.js ~ 14Kb) - test run request fails.

I don't expect WPT issue to be resolved soon.
So I think of launching first version with limited size support only. And later introduce support of POST requests.

@rviscomi @pmeenan do you have alternative ideas/comments?

[pmeenan]

Could also post to the API directly. Though it's more work to implement the polling. Or fork the project and implement post (might be easier).

…

On Thu, Jul 27, 2023 at 3:59 AM Max Ostapenko ***@***.***> wrote: Unfortunately WebPageTest/webpagetest-api doesn't support POST requests to WPT API. So when we try to attach custom metrics code (privacy.js ~ 14Kb) - test run request fails. I don't expect WPT issue <catchpoint/WebPageTest.api-nodejs#177> to be resolved soon. So I think of launching first version with limited size support only. And later introduce support of POST requests. @rviscomi <https://github.com/rviscomi> @pmeenan <https://github.com/pmeenan> do you have alternative ideas/comments? — Reply to this email directly, view it on GitHub <#89 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADMOBJD3VTHRSVW2DS2GNDXSJCYXANCNFSM6AAAAAA2VO7X5Q> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[max-ostapenko]

Thanks @pmeenan
I think I see the easy way to make a fork and probably even make a PR to WPT.

[max-ostapenko]

I tested API as follows:

curl --location 'https://www.webpagetest.org/runtest.php?url=https%3A%2F%2Fexample.com%2F&f=json&k=INSERT_WPT_KEY' \
--header 'Content-Type: application/json' \
--data '{
    "location": "Dulles:Chrome.Native",
    "mobile": 1
}'

but still got mobile: 0 and connectivity: Cable in the test results.

@pmeenan Did I misunderstood the doc and runtest.php doesn't look at parameters passed in POST request body, just URL query?
OR am I doing something wrong?

[tunetheweb]

LGTM once we get the keys setup.

Thanks SOOO much for working on this.

[pmeenan]

I have an API key allocated for this action for use with the instance at webpagetest.httparchive.org. I currently have it assigned to the name HA_API_KEY but I can update the existing WPT_API_KEY secret if that would work better. Just need to change the action to point to the other server (or to read the server from a config).

[github-actions]

Custom metrics for https://almanac.httparchive.org/en/2022/

WPT test run results: http://webpagetest.httparchive.org/results.php?test=231017_P5_6

Custom metrics for https://example.com/

WPT test run results: http://webpagetest.httparchive.org/results.php?test=231017_E9_7
Changed custom metrics values:

{
    "_ads": {
        "ads": {
            "present": false,
            "status": 404
        },
        "app_ads": {
            "present": false,
            "status": 404
        },
        "sellers": {
            "present": false,
            "redirected": false,
            "status": 404
        }
    },
    "_privacy": {
        "privacy_wording_links": [],
        "iab_tcf_v1": {
            "present": false,
            "data": null,
            "compliant_setup": null
        },
        "iab_tcf_v2": {
            "present": false,
            "data": null,
            "compliant_setup": null
        },
        "iab_usp": {
            "present": false,
            "privacy_string": null
        },
        "ads_transparency_spotlight": {
            "present": false,
            "ads_metadata": null
        },
        "document_interestCohort": false,
        "navigator_doNotTrack": false,
        "navigator_globalPrivacyControl": false,
        "document_permissionsPolicy": false,
        "document_featurePolicy": false,
        "referrerPolicy": {
            "entire_document_policy": null,
            "individual_requests": null,
            "link_relations": null
        },
        "media_devices": {
            "navigator_mediaDevices_enumerateDevices": false,
            "navigator_mediaDevices_getUserMedia": false,
            "navigator_mediaDevices_getDisplayMedia": false
        },
        "geolocation": {
            "navigator_geolocation_getCurrentPosition": false,
            "navigator_geolocation_watchPosition": false
        }
    }
}

[max-ostapenko]

@rviscomi no open discussions so far. Ready to merge?

[rviscomi]

LGTM! Thank you so much @max-ostapenko!

[tunetheweb]

Awesome job @max-ostapenko!

[rviscomi]

Looks like this failed on a recent PR: https://github.com/HTTPArchive/custom-metrics/actions/runs/6591822240/job/17911210018?pr=96

Errors thrown in tests/unit-tests.test.js
    ● _ads parsing
  
      thrown: Object {
        "statusCode": 400,
        "statusText": "An error occurred processing your request (missing API key). If you do not have an API key you can purchase one here: https://product.webpagetest.org/api",
      }

[max-ostapenko]

@rviscomi Please confirm that HA_API_KEY secret (the WPT token) is available.
In the debug logs a job gets null.

[rviscomi]

GitHub doesn't actually show the value of the secret, but it seems like it's still there. @pmeenan could you try reapplying it?

[tunetheweb]

Hmmmm this may be a flaw in our plans!

By default, secrets are deliberately not available to PRs from forks. Otherwise an attacker could just create a script to get the secret and open a PR with that script in it.

So not sure how we can get around this...

[tunetheweb]

Did a little research and this seems like a decent way of handling this, though it does require maintainers to rerun a workflow after reviewing the code. It could maybe also be triggered by adding a label since non-maintainers are not allowed to add labels.

One other thing we could do is add certain trusted people that regularly contribute (Aurora and Galaxy teams and Thomas Steiner and Almanac analyst group) to this repo and ask them to open the PRs from this repo rather than the forks to avoid the issue for them. That would avoid the need to rerun or use labels for frequent contributors.

[max-ostapenko]

Yeah, we need to use pull_request_target trigger.
Will open a PR.

One more idea:
We could fetch only particular sources from head branch.
As we read and pass dist/*.js sources to WPT without executing them, it may be be safe to run WPT for websites from PR description for everyone.
Let me know if this would be valuable (or point to safety issues).

BTW all first time contributors already require approval to run workflows by default.

[tunetheweb]

We could fetch only particular sources from head branch.
As we read and pass dist/*.js sources to WPT without executing them, it may be be safe to run WPT for websites from PR description for everyone.

Yeah agree that should be fine. In fact because none of the code is executed locally I think the security risk here is low so think we can checkout all the code from head and no need to limit it. In which case can do it for first run too if using pull_request_target I think.

The main risk is overwhelming our WebPageTest instance and/or the sites themselves with multiple tests but given we limit it to one worker (I think - @pmeenan can you confirm?) and that new users will require approval as you see there doesn't seem huge risk there either.

[max-ostapenko]

PR opened #97

[max-ostapenko]

Some more workflow errors - need to adjust PR description parsing.

[tunetheweb]

Good that it ran though!