Skip to content

enable complex arguments for "npm run render-goldens" command#4542

Merged
elalish merged 17 commits intogoogle:masterfrom
bhouston:update-screenshots-cli
Oct 27, 2023
Merged

enable complex arguments for "npm run render-goldens" command#4542
elalish merged 17 commits intogoogle:masterfrom
bhouston:update-screenshots-cli

Conversation

@bhouston
Copy link
Contributor

@bhouston bhouston commented Oct 26, 2023

This implements this idea: #4536

Options:
      --version       Show version number                              [boolean]
  -c, --config        Path to configuration json             [string] [required]
  -r, --renderer      Limit to specific renderers                        [array]
  -s, --scenario      Limit to specific scenarios                        [array]
  -p, --port          Port for web server               [number] [default: 9040]
  -m, --missing-only  Only render if an output image is missing
                                                      [boolean] [default: false]
  -d, --dry-run       Lists which images would be rendered but doesn't render
                                                      [boolean] [default: false]
  -q, --quiet         Hide the puppeteer controlled browser
                                                      [boolean] [default: false]
  -h, --help          Show help                                        [boolean]

To run a subset of renders or scenarios do something like this:

% npm run render-goldens -- --renderer=filament --renderer=model-viewer --scenario=clearcoat -q 

Explanation of the new features:
--renderer, this now allows you to specify multiple renderers in the whitelist, rather than only one.
-- scenario, this now also allows you to specify multiple scenarios in the whitelist, rather than only one. It now also selects scenarios based on being a subset of the config.json scenario name, thus you can run all tests that contain say "transform" or "clearcoat".
--missing-only is very useful when I add new tests and I don't want to render all other tests just to get goldens for the new ones.
--dry-run, when I want to figure out which tests will run given that command line.
--quiet, I can run update-screenshots in the background while working without Chrome constantly popping up to disrupt my flow. Also gives about a 10% speed boost on my machine.
--port, allows me to run two of these at the same time, useful when I want to run two different renderers at the same time, or work on two separate branches simultaneously.

@bhouston bhouston changed the title enable complex line arguments for update-renderer command enable complex arguments for update-renderer command Oct 26, 2023
@bhouston bhouston changed the title enable complex arguments for update-renderer command enable complex arguments for update-screenshots command Oct 26, 2023
Copy link
Contributor

@elalish elalish 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 great, but can you update our README with the descriptions from your comment? I'm going to give you the benefit of the doubt that you've tested this yourself, since you're using it more than I am at the moment anyway.

@bhouston
Copy link
Contributor Author

bhouston commented Oct 27, 2023

@elalish I've updated the README for "npm run render-goldens."

@bhouston
Copy link
Contributor Author

I've merged in the "npm run test" CLI update into this PR to make it simpler. I've also further updated the README to contain both sets of options in it.

@bhouston bhouston changed the title enable complex arguments for update-screenshots command enable complex arguments for "npm run render-goldens" command Oct 27, 2023
@bhouston
Copy link
Contributor Author

bhouston commented Oct 27, 2023

I've renamed "npm run update-screenshots" to "npm run render-goldens", it is much more accurate/descriptive, e.g. the script renders the images that go into the goldens folder.

@bhouston
Copy link
Contributor Author

In the latest push I've made it so that the renderer you are testing for fidelity is configurable. It defaults to "model-viewer" (for full backwards compatibility) but you can specify any of the web-based renderers (filament, babylon, gltf-sample-viewer, three-gpu-renderer) and it works.

@bhouston bhouston requested a review from elalish October 27, 2023 14:13
Copy link
Contributor

@elalish elalish left a comment

Choose a reason for hiding this comment

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

Thank you, this looks like a huge improvement in usability!

-r, --renderer | | Limit to specific renderers. This now allows you to specify multiple renderers in the whitelist, rather than only one.
-s, --scenario | | Limit to specific scenarios. This now also allows you to specify multiple scenarios in the whitelist. You can use a full name or a partial name of scenarios and it will match against all that contain that scenario substring.
-p, --port | 9040 | Port for web server.
-m, --missing-only | false | Only render if an output image is missing. Very useful when adding new tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@elalish elalish merged commit 9e475c8 into google:master Oct 27, 2023
JL-Vidinoti pushed a commit to vidinoti/model-viewer that referenced this pull request Apr 22, 2024
…#4542)

* enable complex line arguments for update-renderer command, including support for --missing-only, --quiet and --dry-run

* remove debug messages.

* allow setting the port.

* support --scenario being a subset of the scenario name and case insensitive

* add cli for render fidelity tests, allow dry run, quiet modes.

* better description

* enable quite mode in CI.  add missing types for yargs

* improve readme for new update-screenshots cli.

* update readme with new "npm run test" CLI options.

* rename "npm run update-screenshots" to "npm run render-goldens", more accurate name.

* make which renderer you fidelity testing a CLI parameter, defaults to "model-viewer"

* be clear which renderers are supported, just web-based ones.

* update readme with new renderer choice for fidelity test.

* remove debug code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants