phetsims / chipper

Tools for developing and building PhET interactive simulations.
MIT License
11 stars 13 forks source link

We need a better way to prevent string key deletions on main #1446

Open zepumph opened 3 days ago

zepumph commented 3 days ago

It is well known that common code strings files on main need to include all keys for all translations in all supported release branches. Unfortunately this is not being done, and it is not being validated. When we delete a key from main, rosetta silently ignores that key, and often deletes any current translations that have been done to that string key (i.e. https://github.com/phetsims/babel/commit/ffb1a5296f0693b709bfb98dd73082ffd237e6d5). From https://github.com/phetsims/babel/issues/24, it is clear to me that our project is too big to continue with our current process of validation (telling people not to do something that is complicated and hard to know when you are doing).

To illustrate the point, I made a script! This script shows how many errors we currently have in release branches. Not great! Adding to dev meeting to try to decide how to proceed. At the very least we need a loud error when something like this happens.

zepumph commented 3 days ago

This isn't complete, and I'm still working out some bugs in the script, but here it is so far (UPDATED: Edited script and results below):

Problem strings and the sims that are effected ```js { 'JOIST/menuItem.mailInputEventsLog': [ 'atomic-interactions 1.2 phet', 'balancing-chemical-equations 1.2 phet', 'balloons-and-static-electricity 1.5 phet', 'blackbody-spectrum 1.0 phet', 'build-a-fraction 1.0 phet', 'build-a-molecule 1.0 phet', 'capacitor-lab-basics 1.6 phet,phet-io', 'collision-lab 1.1 phet', 'coulombs-law 1.0 phet', 'curve-fitting 1.0 phet', 'diffusion 1.0 phet', 'energy-forms-and-changes 1.4 phet,phet-io', 'equality-explorer-basics 1.0 phet', 'equality-explorer-two-variables 1.0 phet', 'faradays-law 1.4 phet', 'fourier-making-waves 1.0 phet', 'fraction-matcher 1.2 phet', 'fractions-equality 1.1 phet', 'fractions-intro 1.0 phet', 'fractions-mixed-numbers 1.0 phet', 'gas-properties 1.0 phet', 'gases-intro 1.0 phet', 'gravity-force-lab 2.2 phet,phet-io', 'gravity-force-lab-basics 1.1 phet', 'isotopes-and-atomic-mass 1.1 phet', 'john-travoltage 1.6 phet', 'masses-and-springs 1.0 phet', 'masses-and-springs-basics 1.0 phet', 'molarity 1.5 phet', 'molecules-and-light 1.5 phet', 'ohms-law 1.4 phet', 'reactants-products-and-leftovers 1.2 phet', 'resistance-in-a-wire 1.6 phet', 'rutherford-scattering 1.1 phet', 'states-of-matter 1.2 phet,phet-io', 'states-of-matter-basics 1.2 phet,phet-io', 'vector-addition 1.0 phet', 'vector-addition-equations 1.0 phet', 'wave-interference 2.0 phet', 'waves-intro 1.1 phet' ], 'JOIST/menuItem.outputInputEventsLog': [ 'atomic-interactions 1.2 phet', 'balancing-chemical-equations 1.2 phet', 'balloons-and-static-electricity 1.5 phet', 'blackbody-spectrum 1.0 phet', 'build-a-fraction 1.0 phet', 'build-a-molecule 1.0 phet', 'capacitor-lab-basics 1.6 phet,phet-io', 'collision-lab 1.1 phet', 'coulombs-law 1.0 phet', 'curve-fitting 1.0 phet', 'diffusion 1.0 phet', 'energy-forms-and-changes 1.4 phet,phet-io', 'equality-explorer-basics 1.0 phet', 'equality-explorer-two-variables 1.0 phet', 'faradays-law 1.4 phet', 'fourier-making-waves 1.0 phet', 'fraction-matcher 1.2 phet', 'fractions-equality 1.1 phet', 'fractions-intro 1.0 phet', 'fractions-mixed-numbers 1.0 phet', 'gas-properties 1.0 phet', 'gases-intro 1.0 phet', 'gravity-force-lab 2.2 phet,phet-io', 'gravity-force-lab-basics 1.1 phet', 'isotopes-and-atomic-mass 1.1 phet', 'john-travoltage 1.6 phet', 'masses-and-springs 1.0 phet', 'masses-and-springs-basics 1.0 phet', 'molarity 1.5 phet', 'molecules-and-light 1.5 phet', 'ohms-law 1.4 phet', 'reactants-products-and-leftovers 1.2 phet', 'resistance-in-a-wire 1.6 phet', 'rutherford-scattering 1.1 phet', 'states-of-matter 1.2 phet,phet-io', 'states-of-matter-basics 1.2 phet,phet-io', 'vector-addition 1.0 phet', 'vector-addition-equations 1.0 phet', 'wave-interference 2.0 phet', 'waves-intro 1.1 phet' ], 'JOIST/menuItem.submitInputEventsLog': [ 'atomic-interactions 1.2 phet', 'balancing-chemical-equations 1.2 phet', 'balloons-and-static-electricity 1.5 phet', 'blackbody-spectrum 1.0 phet', 'build-a-fraction 1.0 phet', 'build-a-molecule 1.0 phet', 'capacitor-lab-basics 1.6 phet,phet-io', 'collision-lab 1.1 phet', 'coulombs-law 1.0 phet', 'curve-fitting 1.0 phet', 'diffusion 1.0 phet', 'energy-forms-and-changes 1.4 phet,phet-io', 'equality-explorer-basics 1.0 phet', 'equality-explorer-two-variables 1.0 phet', 'faradays-law 1.4 phet', 'fourier-making-waves 1.0 phet', 'fraction-matcher 1.2 phet', 'fractions-equality 1.1 phet', 'fractions-intro 1.0 phet', 'fractions-mixed-numbers 1.0 phet', 'gas-properties 1.0 phet', 'gases-intro 1.0 phet', 'gravity-force-lab 2.2 phet,phet-io', 'gravity-force-lab-basics 1.1 phet', 'isotopes-and-atomic-mass 1.1 phet', 'john-travoltage 1.6 phet', 'masses-and-springs 1.0 phet', 'masses-and-springs-basics 1.0 phet', 'molarity 1.5 phet', 'molecules-and-light 1.5 phet', 'ohms-law 1.4 phet', 'reactants-products-and-leftovers 1.2 phet', 'resistance-in-a-wire 1.6 phet', 'rutherford-scattering 1.1 phet', 'states-of-matter 1.2 phet,phet-io', 'states-of-matter-basics 1.2 phet,phet-io', 'vector-addition 1.0 phet', 'vector-addition-equations 1.0 phet', 'wave-interference 2.0 phet', 'waves-intro 1.1 phet' ], 'SCENERY_PHET/ResetAllButton.name': [ 'balancing-chemical-equations 1.2 phet', 'build-a-fraction 1.0 phet', 'capacitor-lab-basics 1.6 phet,phet-io', 'coulombs-law 1.0 phet', 'equality-explorer-basics 1.0 phet', 'equality-explorer-two-variables 1.0 phet', 'fractions-intro 1.0 phet', 'fractions-mixed-numbers 1.0 phet', 'masses-and-springs 1.0 phet', 'reactants-products-and-leftovers 1.2 phet', 'resistance-in-a-wire 1.6 phet', 'rutherford-scattering 1.1 phet' ], 'JOIST/preferences.tabs.input.gestureControls.title': [ 'balloons-and-static-electricity 1.5 phet', 'circuit-construction-kit-ac 1.0 phet', 'circuit-construction-kit-ac-virtual-lab 1.0 phet', 'equality-explorer 1.1 phet', 'fourier-making-waves 1.0 phet', 'gravity-force-lab-basics 1.1 phet', 'john-travoltage 1.6 phet', 'normal-modes 1.0 phet' ], 'JOIST/preferences.tabs.input.gestureControls.description': [ 'balloons-and-static-electricity 1.5 phet', 'circuit-construction-kit-ac 1.0 phet', 'circuit-construction-kit-ac-virtual-lab 1.0 phet', 'equality-explorer 1.1 phet', 'fourier-making-waves 1.0 phet', 'gravity-force-lab-basics 1.1 phet', 'john-travoltage 1.6 phet', 'normal-modes 1.0 phet' ], 'JOIST/preferences.tabs.general.simulationSpecificSettings': [ 'balloons-and-static-electricity 1.5 phet', 'circuit-construction-kit-ac 1.0 phet', 'circuit-construction-kit-ac-virtual-lab 1.0 phet', 'equality-explorer 1.1 phet', 'fourier-making-waves 1.0 phet', 'gravity-force-lab-basics 1.1 phet', 'john-travoltage 1.6 phet', 'normal-modes 1.0 phet' ], 'DENSITY_BUOYANCY_COMMON/average': [ 'density 1.1 phet,phet-io' ], 'SHRED/ununbium': [ 'isotopes-and-atomic-mass 1.1 phet', 'rutherford-scattering 1.1 phet' ], 'SHRED/ununtrium': [ 'isotopes-and-atomic-mass 1.1 phet', 'rutherford-scattering 1.1 phet' ], 'SHRED/ununpentium': [ 'isotopes-and-atomic-mass 1.1 phet', 'rutherford-scattering 1.1 phet' ], 'SHRED/ununseptium': [ 'isotopes-and-atomic-mass 1.1 phet', 'rutherford-scattering 1.1 phet' ], 'SHRED/ununoctium': [ 'isotopes-and-atomic-mass 1.1 phet', 'rutherford-scattering 1.1 phet' ] } ```
script ```js /** * Usage: cd repo/, node "this file" */ process.chdir( '../perennial' ); const puppeteerLoad = require( '../perennial/js/common/puppeteerLoad' ); const Maintenance = require( '../perennial/js/common/Maintenance' ); const withServer = require( '../perennial/js/common/withServer' ); const puppeteer = require( '../perennial/node_modules/puppeteer' ); const fs = require( 'fs' ); const _ = require( 'lodash' ); const winston = require( '../perennial/node_modules/winston' ); winston.default.transports.console.level = 'error'; ( async () => { const browser = await puppeteer.launch( { args: [ '--disable-gpu' ] } ); // Use withServer for cross-dev environment execution. await withServer( async port => { const getStringMap = async releaseBranch => { const url = `http://localhost:${port}/release-branches/${releaseBranch.repo}-${releaseBranch.branch}/${releaseBranch.repo}/build/phet/${releaseBranch.repo}_all_phet.html`; return puppeteerLoad( url, { evaluate: () => { return phet.chipper.strings; }, waitForFunction: '!!phet?.chipper?.strings', gotoTimeout: 60000, waitAfterLoad: 2000, allowedTimeToLoad: 40000, browser: browser // , // eslint-disable-line comma-style // logConsoleOutput: true, // logger: console.log } ); }; const releaseBranches = await Maintenance.loadAllMaintenanceBranches(); // Record const problemStrings = {}; const populateProblem = ( stringKey, releaseBranch ) => { problemStrings[ stringKey ] = problemStrings[ stringKey ] || []; problemStrings[ stringKey ].push( releaseBranch.toString() ); }; // Record const mapOnMain = {}; const getFromCache = requireJSNamespace => { if ( !mapOnMain.hasOwnProperty( requireJSNamespace ) ) { const repo = _.kebabCase( requireJSNamespace ); mapOnMain[ requireJSNamespace ] = JSON.parse( fs.readFileSync( `../${repo}/${repo}-strings_en.json` ) ); } return mapOnMain[ requireJSNamespace ]; }; for ( const releaseBranch of releaseBranches ) { console.log( releaseBranch.toString() ); const pathRoot = `../release-branches/${releaseBranch.repo}-${releaseBranch.branch}/`; const packageJSON = JSON.parse( fs.readFileSync( `${pathRoot}/${releaseBranch.repo}/package.json` ) ); const simRequireJSNamespace = packageJSON.phet.requirejsNamespace; let stringMap; try { stringMap = await getStringMap( releaseBranch ); } catch( e ) { if ( e.message.includes( 'phet is not defined' ) ) { console.log( 'Need to update release branch build' ); } else { console.error( e ); } continue; } if ( !stringMap.hasOwnProperty( 'en' ) ) { throw new Error( `no en for sim: ${releaseBranch.repo}` ); } for ( const stringKey of Object.keys( stringMap.en ) ) { const keyParts = stringKey.split( '/' ); const requireJSNamespace = keyParts[ 0 ]; const actualKey = keyParts.slice( 1, keyParts.length ).join( '/' ); if ( requireJSNamespace !== simRequireJSNamespace && !actualKey.startsWith( 'a11y' ) ) { const mainMap = getFromCache( requireJSNamespace ); if ( !mainMap.hasOwnProperty( actualKey ) ) { populateProblem( stringKey, releaseBranch ); console.log( problemStrings ); } } } } } ); browser.close(); } )(); ```
zepumph commented 3 days ago
``` JOIST/preferences.tabs.localization.regionAndCulture.portrayalSets.africa JOIST/preferences.tabs.localization.regionAndCulture.portrayalSets.africaModest JOIST/preferences.tabs.localization.regionAndCulture.portrayalSets.asia JOIST/preferences.tabs.localization.regionAndCulture.portrayalSets.latinAmerica JOIST/preferences.tabs.localization.regionAndCulture.portrayalSets.oceania JOIST/preferences.tabs.localization.regionAndCulture.portrayalSets.multicultural JOIST/preferences.tabs.localization.regionAndCulture.portrayalSets.random JOIST/preferences.tabs.localization.regionAndCulture.portrayalSets.unitedStatesOfAmerica SOCCER_COMMON/characterSet.unitedStatesOfAmerica SOCCER_COMMON/characterSet.africa SOCCER_COMMON/characterSet.africaModest ```