Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Puppeteer E2E test: many changes #24109

Closed
wants to merge 124 commits into from

Conversation

LeviPesin
Copy link
Contributor

@LeviPesin LeviPesin commented May 23, 2022

Related issue: -

Description

Many changes to the Puppeteer E2E test and tests in general (e.g. lint test).

  • Changed the progressive attempts to "ordinary" attempts (they all have the same network and render timeouts) because that allows to increase "mean" timeouts and still works if a random unrelated to the timeouts error occurs on one of the attempts. And it seems it is faster now :-),
  • Removed pageSize code - I think it does not need such fine-tuning, and it actually never worked - the timeout passed to setTimeout was negative in most cases because Math.min was used there instead of Math.max,
  • Now timeouts can be set to 0 to disable them,
  • Now the test autodownloads the latest browser and uses puppeteer-core instead of puppeteer,
  • Because puppeteer-core is much smaller (it does not include the browser), I merged test/package.json with package.json,
  • Changed the stucture of output of the test (now it also includes the errors and warning thrown by the examples, which will allow to autodetect errors - and it already detects them, e.g. a use of RGBFormat somewhere),
  • Fixed WebGPU.

TODO:

  • Possibly remove the attempts entirely and instead do one attempt but with large timeouts? This can possibly break if an error unrelated to the timeouts happens, I actually think this is not needed - errors happen rarely, so the second attempt would be rarely needed, so it would not result in slow tests,
  • Configure timeouts,
  • Investigate whether all the flags are still needed,
  • Retest examples remaining in the exception list (I think that my timeout fixes should fix some of them),
  • Investigate whether GitHub Actions allow to autosupply Chromium to the E2E test (so that it would not be required for it to download), (is not needed, GitHub network is very fast)
  • Remove on-the-fly changing of build/three.module.js (this requires regenerating most of the screenshots for some reason, I think?),
  • Investigate is there a way to NOT cancel the remaining tests if one of them fails,
  • Make a list of browser warnings and errors found by the test,
  • Check whether all examples match their screenshots (something may be not picked up by the Puppeteer, that happened e.g. in the webgl_loader_imagebitmap),
  • Use parallel pages (and then use one job instead of multiple),
  • Use multiple OSes for testing (and separate tests for that),
  • Possibly use multiple browsers for testing (and separate tests for that),
  • Possibly make screenshot and test with different options of GUI (to cover more cases in examples),
  • Make lint test also cover examples and docs directories,
  • Fix WebGPU,
  • Make a test testing editor and documentation,
  • Use DPR for viewScale.

Current problems:

  • Fix parallelism fragility,
  • Fix script stopping on browser closing,
  • Fix WebGPU on Linux, (the problem is not in Linux, but in GitHub Actions)
  • Fix shadow in the top for fromSurface: false screenshoting,
  • Fix video determinism,
  • Remove window.TESTING,
  • Fix examples remaining in the exception list,
  • Refactor idleTime and parseTime system (revert to one that was before and add per-example custom timeouts?). (is not needed)
  • Allow to download output-screenshots/ directory from tests on GitHub.

Strange errors:

webxr_vr_sandbox: WebGL: INVALID_VALUE: texSubImage2D: no canvas

webgpu_instance_uniform: THREE.MeshStandardMaterial: '_listeners' is not a property of this material.

(the following errors happen rarely)

webgl_materials_normalmap: THREE.WebGLProgram: Program Info Log: C:\fakepath(53,12-96): warning X4713: Sample Bias value is limited to the range [-16.00, 15.99], using -16.000000 instead of -100.000000
C:\fakepath(53,12-96): warning X4713: Sample Bias value is limited to the range [-16.00, 15.99], using -16.000000 instead of -100.000000
C:\fakepath(53,12-96): warning X4713: Sample Bias value is limited to the range [-16.00, 15.99], using -16.000000 instead of -100.000000
C:\fakepath(53,12-96): warning X4713: Sample Bias value is limited to the range [-16.00, 15.99], using -16.000000 instead of -100.000000
C:\fakepath(53,12-96): warning X4713: Sample Bias value is limited to the range [-16.00, 15.99], using -16.000000 instead of -100.000000
C:\fakepath(53,12-96): warning X4713: Sample Bias value is limited to the range [-16.00, 15.99], using -16.000000 instead of -100.000000
C:\fakepath(53,12-96): warning X4713: Sample Bias value is limited to the range [-16.00, 15.99], using -16.000000 instead of -100.000000

webgl_raymarching_reflect: THREE.WebGLProgram: Program Info Log: C:\fakepath(137,24-47): warning X4008: floating point division by zero
C:\fakepath(137,24-47): warning X4008: floating point division by zero
C:\fakepath(137,24-47): warning X4008: floating point division by zero
C:\fakepath(137,24-47): warning X4008: floating point division by zero
C:\fakepath(137,24-47): warning X4008: floating point division by zero

webgl2_materials_texture3d: THREE.WebGLProgram: Program Info Log: C:\fakepath(312,3-49): warning X3557: loop only executes for 1 iteration(s), forcing loop to unroll

webgl_shaders_tonemapping: THREE.WebGLProgram: Program Info Log: C:\fakepath(99,3-49): warning X3557: loop only executes for 1 iteration(s), forcing loop to unroll

webgl_buffergeometry_lines: THREE.WebGLProgram: Program Info Log: C:\fakepath(127,3-49): warning X3557: loop only executes for 1 iteration(s), forcing loop to unroll

@mrdoob
Copy link
Owner

mrdoob commented May 24, 2022

Seems like on Linux WebGPU can be enabled by launching Chrome with --enable-features=Vulkan.
https://stackoverflow.com/a/70111365

@LeviPesin
Copy link
Contributor Author

But it still does not work on Windows...

test/e2e/puppeteer.js Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@LeviPesin
Copy link
Contributor Author

LeviPesin commented Jul 6, 2022

Hm, should I split my changes to the lint test (there are not too much of them, actually) to a separate PR?

@mrdoob
Copy link
Owner

mrdoob commented Jul 7, 2022

Hmm, yeah. It'd be great if you can do a different PR for the lint fixes if possible.

@LeviPesin
Copy link
Contributor Author

LeviPesin commented Jan 20, 2023

I've decided to split this PR into multiple small PRs (both for easier resolving conflicts and making easier to merge), for example:

  • Making the test download Chrome (and removing test/package-lock.json),
  • Adding output from console to the test's output,
  • Testing on multiple OSes,
  • and etc...

Will file them soon. (Closing this PR.)

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.

None yet

5 participants