Use puppeteer-core with custom chromium instead of puppeteer #222
Conversation
Fix workflow
Fix workflow Update workflows Update workflows
| cookie?: SetCookie | ||
| cookie?: SetCookie, | ||
| chromiumPath = CHROMIUM_PATH |
There was a problem hiding this comment.
can you put the optional parameter as the last parameter? So that in the test file, I don't think you need to pass in undefined anymore
There was a problem hiding this comment.
I don't know where the cookie parameter is used, if I swap them whenever you pass in cookie you'll have to pass undefined or chromium path before cookie
There was a problem hiding this comment.
In your code, the chromium parameter already has an initializer which uses CHROMIUM_PATH, besides test, I don't think we need to pass in different value for the path
There was a problem hiding this comment.
Oh I see your point, that won't work. Never mind, ignore my comment
There was a problem hiding this comment.
BTW I tried to mock CHROMIUM_PATH and used jest.requireActual to import other constants in the test file. The constants are correct in the test file, but in visualReportHelper.ts only CHROMIUM_PATH is correct.
That's why I used the optional parameter workaround, if you are more familiar with jest you can try to mock the constant which would be a better solution
|
Should we provide the different chromium version somewhere for developers? mac, windows and linux? |
they are currently in a release under reporting https://github.com/opendistro-for-elasticsearch/kibana-reports/releases/tag/chromium-1.12.0.0 |
…tro-for-elasticsearch#222) * Use custom chrome binary * Update chromium path * Test workflow for al2 * Disable tests temporarily * Fix workflow Fix workflow * Rename opendistroKibanaReports to opendistroReportsKibana * Update workflows Fix workflow Update workflows Update workflows * Enable tests in workflows * Add windows chromium to workflows * Fix tests * Use path.posix.normalize to accomondate windows * Use @types/puppeteer-core
…tro-for-elasticsearch#222) * Use custom chrome binary * Update chromium path * Test workflow for al2 * Disable tests temporarily * Fix workflow Fix workflow * Rename opendistroKibanaReports to opendistroReportsKibana * Update workflows Fix workflow Update workflows Update workflows * Enable tests in workflows * Add windows chromium to workflows * Fix tests * Use path.posix.normalize to accomondate windows * Use @types/puppeteer-core
Issue #, if available:
Description of changes:
opendistroKibanaReportstoopendistroReportsKibanafor consistencycreateVisualReportfor jest testingpath.posix.normalizeinstead ofpath.normalizewhen verifying URL to accommodate WindowsCaveats
Chromium path defaults to(Fixed by Use chromium path relative to constant file #236 )./plugins/opendistroReportsKibana/.chromium/headless_shellwhich works for the zip artifact, but if running from source it should be./plugins/kibana-reports/.chromium/headless_shellBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.