first stab at templating app_domain and app_name strings#627
Conversation
|
|
||
| # Set absolute paths for tests (relative paths break when ChromeDriver spawns Chrome) | ||
| echo "GOOGLE_CHROME_BINARY=$GITHUB_WORKSPACE/chrome-linux64/chrome" >> $GITHUB_ENV | ||
| echo "CHROMEDRIVER_BINARY=$GITHUB_WORKSPACE/chromedriver-linux64/chromedriver" >> $GITHUB_ENV |
There was a problem hiding this comment.
I'm pretty sure this is breaking the CI.
What was failing? Like what's the “Claude says”? This dynamic “fetch-chromedriver” has been working for years.
There was a problem hiding this comment.
Here's Claude's reasoning behind the fix (copy/pasted):
--
- Relative → absolute path: ./chrome-linux64/chrome → $GITHUB_WORKSPACE/chrome-linux64/chrome. ChromeDriver spawns Chrome as a subprocess and the working directory isn't guaranteed to be the repo root.
- ChromeDriver extracted and wired up directly: Now we unzip chromedriver-linux64.zip and set CHROMEDRIVER_BINARY so util.js uses chrome.setDefaultService() with the exact binary, bypassing the npm chromedriver package entirely (and those two npm install chromedriver steps that were adding noise).
- --disable-dev-shm-usage added: Chrome in Docker/CI containers shares /dev/shm by default (64MB limit) which is too small and causes crashes. This flag disables that.
CC @schanzer Turns out this was "working" because package-lock.json was out of date, so the commands that were present were trying to update. This forces the rebuild (which was already kind of what was happening in #627 (comment)) *and* makes updates to util.js so that the environment variables work out. (I think) the problem with the PR was that util was running with the default headless chrome in the CI build, which is not the one that was just unzipped
This reverts commit 5771215.
|
Thanks! I'm trying to fix the CI directly on horizon. I reverted the CI fix that's here. |
| <title>{{APP_NAME}}</title> | ||
| <link crossorigin="anonymous" rel="preload" href="{{&PYRET}}" as="script"> | ||
| <script>window.PYRET = "{{&PYRET}}";</script> | ||
| <script>window.PYRET = "{{&PYRET}}"; window.APP_NAME = "{{APP_NAME}}"; window.APP_DOMAIN = "{{APP_DOMAIN}}";</script> |
There was a problem hiding this comment.
Do we need the & here? Any escaping that we should include/avoid? Should probably justify the difference.
| <head> | ||
| <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/> | ||
| <title>code.pyret.org</title> | ||
| <title>{{APP_NAME}}</title> |
There was a problem hiding this comment.
Dumb question: does faq get templated by server.js? I forget which pages actually get all the template variables. Is it all of them?
There was a problem hiding this comment.
Looks like it doesn't:
app.get("/close.html", function(_, res) { res.render("close.html"); })
Is there any reason not to pass defaultOpts to all of them?
2 participants
Trying to template out strings that are user-facing vs. code-facing. This would make diffs with pyret.bootstrapworld.org a little easier, assuming I have the semantics right