[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Page MenuHomePhabricator

Export Popups and MobileFrontend dist files as CI artifacts
Open, MediumPublic

Description

Popupos and MobileFrontend CI builds the dist files from the Javascript sources, and compares those against the dist files in the patch and rejects it if they aren't the same. This is a major barrier to entry and source of frustration for new contributors - it requires significant extra environment setup compared to what's needed for writing and testing the patch, and it's very erratic - sometimes you build the distfiles using the exact same npm and library versions and parent commit, and it still doesn't match.

At a minimum a clearer error message would be nice (right now it says Try running npm run build again or removing the node_modules folder and running npm install with the correct node version., even though it just asserted a few lines above that the node version matches), but maybe a better approach would be to export the dist files as CI artifacts, so instead of struggling with tracking down minuscule differences in their local setup, contributors could just copy the correct dist files from Jenkins into the patch.

Event Timeline

I think it might be worth re-evaluating the use of Webpack in MobileFrontend before committing to this. Since it was introduced ResourceLoader has gone through several improvements. I'm not sure when we will get time to do this, so let us know if it's blocking anything you are working on!

I think it might be worth re-evaluating the use of Webpack in MobileFrontend before committing to this.

@Jdlrobson Is there a task for that? Getting rid of the build step, if possible, would be a nice alternative to this task.

The Popups codebase has the same problem as this presumably and is a much smaller codebase so the idea was to do T315929 first (but there's not been much urgency there) and apply learnings to MobileFrontend. I will update the ticket to be about both (this should probably do the same).

I should note, the lack of a decision in T279108 reduces motivation to do this as there are a lot of advantages to a build step and it would be frustrating to remove it if we then saw momentum for adding a build step.

Tgr renamed this task from Export MobileFrontend dist files as CI artifacts to Export Popups and MobileFrontend dist files as CI artifacts.Dec 12 2023, 5:08 PM
Tgr updated the task description. (Show Details)

We have been talking about whether to have a build step since I joined the WMF so I wouldn't hold my breath there. A CI artifact seems like a low-effort workaround that somewhat mitigates one of the worst drawbacks.

Jdlrobson changed the task status from Open to Stalled.Sep 25 2024, 12:42 AM

Should wait on outcome of T315929

hashar changed the task status from Stalled to Open.Sep 30 2024, 1:21 PM
hashar subscribed.

The CI jobs should capture anything written under a log directory whose path is available via the LOG_DIR environment variable (it is set by CI). Anything written there would end up attached to the Jenkins build and thus available to the user.

An example for the shared config of our Selenium tests:

tests/selenium/wdio-mediawiki/wdio-defaults.conf.js
const logPath = process.env.LOG_DIR || path.join( process.cwd(), 'tests/selenium/log' );
...
exports.config = {
    // Setting this enables automatic screenshots for when a browser command fails
    // It is also used by afterTest for capturing screenshots.
    screenshotPath: logPath,
...

So tentatively on failure, the script could copy the resources/dist directory to LOG_DIR and it will be copied. The script can also output a link to the artifacts directory on the Jenkins primary using the environment variables set by Jenkins:

printf "See the generated bundle at: %s/artifact/log\n" "$BUILD_URL"