phetsims / chipper

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

"grunt create-composite-third-party-report" fails #199

Closed pixelzoom closed 9 years ago

pixelzoom commented 9 years ago

https://github.com/phetsims/chipper/issues/178 says:

Added a new grunt task: create-composite-third-party-report #162 Given the simulation-specific reports created by createSimSpecificThirdPartyReport, aggregate them and provide a complete report, indicating which third-party resources are used by which simulations.

This grunt task fails. Sample output:

% pwd
/Users/cmalley/PhET/GitHub/hookes-law 
% grunt create-composite-third-party-report
Running "create-composite-third-party-report" task
Warning: missing reports for acid-base-solutions, area-builder, arithmetic, atomic-interactions, 
balancing-act, balancing-chemical-equations, balloons-and-static-electricity, beaker, beers-law-lab, 
bending-light, blackbody-spectrum, blast, build-a-molecule, build-an-atom, calculus-grapher, 
capacitor-lab-basics, chains, charges-and-fields, color-vision, concentration, curve-fitting, 
energy-forms-and-changes, energy-skate-park-basics, estimation, example-sim, faradays-law, 
fluid-pressure-and-flow, forces-and-motion-basics, fraction-comparison, fraction-matcher, friction, 
function-builder, gene-expression-basics, graphing-lines, graphing-quadratics, gravity-and-orbits, 
gravity-force-lab, griddle, hookes-law, isotopes-and-atomic-mass, john-travoltage, 
least-squares-regression, litmus, making-tens, mikes-playground, molarity, molecule-polarity, 
molecule-shapes, molecule-shapes-basics, molecules-and-light, neuron, ohms-law, optics-lab, 
pendulum-lab, ph-scale, ph-scale-basics, plinko-probability, projectile-motion, protein-synthesis, 
reactants-products-and-leftovers, resistance-in-a-wire, rutherford-scattering, scenery-phet, seasons, 
simula-rasa, states-of-matter, states-of-matter-basics, sugar-and-salt-solutions, sun, trig-lab, 
under-pressure, vegas, vibe, wave-on-a-string Use --force to continue.

Aborted due to warnings.
%
pixelzoom commented 9 years ago

Perhaps this task needs to be run from a specific directory. That's not indicated in the "grunt --help" or in the source code comment header.

178 asks:

How to handle stale third-party-report.json files?

After looking for third-party-report.json, I discovered that this is probably third-party-report.md, and it's written to sherpa/. This should be indicated in "grunt --help" output for the task.

So I tried running "grunt create-composite-third-party-report" from the sherpa directory. But that can't be the case, because sherpa has no package.json, and therefore can't run "npm install".

samreid commented 9 years ago

From the JSDoc:

Entries in sherpa/license.json are used for the code report, and this is augmented by information obtained by running mandatory simulation-specific reports using:

grunt-all.sh build-no-lint --createSimSpecificThirdPartyReport

indicating which third-party code resources are used by which simulations.

(emphasis added)

pixelzoom commented 9 years ago

[8/3/15, 4:32:19 PM] Chris Malley: I have to manually run " grunt-all.sh build-no-lint --createSimSpecificThirdPartyReport" before using this task? That's not at all clear. [8/3/15, 4:33:13 PM] Sam Reid: Let’s rewrite it to make it clearer. [8/3/15, 4:33:47 PM] Chris Malley: And how do I know that the sim-specific reports that do exist aren't stale? [8/3/15, 4:34:14 PM] Chris Malley: The only way I can be sure is to rebuild them all. [8/3/15, 4:34:29 PM] Chris Malley: So why not just have the composite task do that? [8/3/15, 4:34:42 PM] Chris Malley: I see zero value to reusing what are likely to be stale reports.

samreid commented 9 years ago

It takes 30+ minutes to generate the sim specific reports. It would be very painful if we had to wait 30+ minutes each time we wanted to iterate on the output for the composite report.

pixelzoom commented 9 years ago

I realize the time involved. But the time tradeoff means that the report will almost certainly be inaccurate unless you've run grunt-all.sh before aggregating the results.

samreid commented 9 years ago

Perhaps we can devise a way that will default to running the entire grunt-all.sh, but still allow developers an option to use pre-existing sim-specific reports when necessary. Perhaps one day when the need to iterate quickly over pre-exsting sim reports is gone (if ever) we can remove the option.

samreid commented 9 years ago

What do you recommend as an implementation approach for running grunt-all.sh before running the aggregation task? Brainstorming some ideas:

  1. Entry point is a shell script that invokes grunt-all.sh or grunt-all.sh report-sim-licenses (depending on the decision in https://github.com/phetsims/chipper/issues/207. The shell script could be called generate-composite-license-report.sh
  2. Entry point is a grunt task which invokes grunt-all.sh or grunt-all.sh report-sim-licenses and waits for it to complete.
  3. Entry point is a grunt task which simply fails and tells the user they must run grunt-all.sh or grunt-all.sh report-sim-licenses before continuing. Then the user can run the grunt task with an option like grunt report-composite-licenses --ijustnowgeneratedsimspecificreports=true to get it to continue
  4. Something else
pixelzoom commented 9 years ago

(4) Something else.

See related Skype discussion in https://github.com/phetsims/chipper/issues/207#issuecomment-127783449.

samreid commented 9 years ago

This will be addressed in https://github.com/phetsims/chipper/issues/220, closing.