-
Notifications
You must be signed in to change notification settings - Fork 29
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
Update php container Dockerfile to point at specific version of composer #1836
base: legacy
Are you sure you want to change the base?
Conversation
- add a new npm script to run all tests besides the DoenetML ones - electron runs about 25% faster for me, so I set that as the browser for this new script, to have a quick signal if the app side is likely in a good state. There is a tiny chance chrome behaves differently, but they are both based on Chromium
This isn't ready for immediate merging, I want to get input from others, primarily Emilio first. |
Convert to draft PR? |
@@ -20,7 +20,7 @@ | |||
"buildx:mysql": "docker buildx build --platform linux/amd64 -f doenet_docker/mysql/Dockerfile -t lyanthropos/doenet_test:mysql-dev .", | |||
"test": "cypress open", | |||
"test:all": "cypress run -b 'chrome' --config video=false --headless", | |||
"test:app": "cypress run -b 'electron' --config video=false --headless --spec 'cypress/e2e/ActivityViewer/*,cypress/e2e/AsStudent/*,cypress/e2e/Events/*,cypress/e2e/Navigation/*,cypress/e2e/Public/*,cypress/e2e/AssignedActivity/*,cypress/e2e/course/*,cypress/e2e/Editor/*,cypress/e2e/Gradebook/*,cypress/e2e/People/*'", | |||
"test:app": "cypress run -b 'electron' --config video=false --headless ignoreTestFiles='cypress/e2e/DoenetML/*'", |
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.
apparently this doesn't work, still trying to see if there is a way to do an exclude list rather than an include list with a directory
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.
We could always change the structure so that the DoenetML specs are in one directory and the rest are in another directory so that you can use a simple --spec with one directory.
Eventually, the plan is to move DoenetML into the DoenetML repository, at which point the specs will be separate anyway.
I ran into an issue after cleaning out my containers completely, I had messed things up while trying to backport the LTI stuff to 5.4, after starting out from a clean environment I was getting this failure on requests to the php server. Pinning to this version of composer and rebuilding my container with
npm run buildx:php
fixed it for me. @Lyanthropos do you think pinning a version like this makes sense?Apparently docker does not very proactively pull in new versions of containers that you depend on if it has any version cached locally. Which I guess is the reason I ran into this but everyone else has been fine. Although I am curious about Duane recently setting up his laptop and not hitting this.
https://stackoverflow.com/questions/74141587/does-docker-from-keyword-in-dockerfile-look-for-the-newest-image