-
Notifications
You must be signed in to change notification settings - Fork 83
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
[wip] feat: welcome typescript and cypress in docker #4237
base: main
Are you sure you want to change the base?
Conversation
Thanks a lot for looking into this @skjnldsv ! |
35f44b1
to
aaac274
Compare
|
||
// Before the browser launches | ||
// starting Nextcloud testing container | ||
return startNextcloud(process.env.BRANCH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have an option to still run this towards a local instance for easier debugging. And the currently used docker image doesn't come with arm architecture support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we support arm officially?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on what you mean with supporting, at least community has high demand with single board computers.
Also this would block developers on apple arm chips from running the tests in a reasonable time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least providing a way to just pass CYPRESS_baseUrl as an environment variable and then not setup docker would be nice, but I can look into that part :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I'm guessing the current behaviour also breaks on windows.
I think it's an interesting topic, but I don't think it's worth spending more time on it. 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Hephi2 had problems running the server tests on windows as well with WSL+Docker. Let's go for the manual URL approach then as a fallback/workaround which should be easy to add
await runExec(container, ['php', 'occ', 'config:system:set', 'force_language', '--value', 'en'], true) | ||
await runExec(container, ['php', 'occ', 'config:system:set', 'default_locale', '--value', 'en_US'], true) | ||
await runExec(container, ['php', 'occ', 'config:system:set', 'force_locale', '--value', 'en_US'], true) | ||
await runExec(container, ['php', 'occ', 'config:system:set', 'enforce_theme', '--value', 'light'], true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally also have "memcache.local"=>"\OC\Memcache\APCu","hashing_default_password"=>true
for faster runs
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
aaac274
to
7cd1ae4
Compare
Ongoing effort, some adjustments might be needed 😉
Please take over Text team! 🚀