end2end localdb tests #6839
end2end localdb tests #6839
Conversation
n1zea144
force-pushed
the
fix-3850
branch
10 times, most recently
from
af91aa5
to
54d2812
Compare
| paths: | ||
| - cbioportal-frontend | ||
|
|
||
| install_yarn: |
There was a problem hiding this comment.
Once the e2e tests are using the backend of this PR the install_yarn job prolly won't be necessary anymore because the frontend artifacts are already in the war. No need to recompile them
There was a problem hiding this comment.
There was a problem hiding this comment.
Good point - so it looks like it's pretty much working then 🎉. My point was mostly regarding some minor optimization. I think currently the localdb test tries to mount the frontend inside the war. That's why you need these steps of yarn run build etc. On the frontend e2e tests that makes sense cause you need to test against that frontend code in the PR. On the backend it's redundant b/c the war already contains the compiled frontend.
Another minor thing is that in the localdb tests the backend code gets pulled from GitHub again and the docker image is made. An optimization would be to create the image before running these tests and using it subsequently in the localdb tests ( to avoid the extra pull). You can then extend it with things like: only rebuild the image when the code has changed.
But yeah we can skip these optimizations for now I think, better to have something working in master branch.
Maybe just double check the backend of the PR is actually being used e.g. by introducing an error or something. And check it is properly creating the new backend image when you add a new commit in the pr to fix it.
There was a problem hiding this comment.
It is true that the only use of the backend checkout right now is to get the version of the frontend to checkout. I explored the idea of creating an image from the checkout step done by the pull_frontend_codebase job. In one scenario, it may just involve the modification of build_portal_image.sh, maybe with some additional args passed to setup_docker_containers.sh (called by the circle-ci config). @pvannierop any thoughts on this?
I did see from the circle-ci page that my hash was being used during the Setup docker images and containers step. I went ahead and introduced an error as you suggested to confirm the error and fix on revert. This looks good.
There was a problem hiding this comment.
@inodb I have a further thought on this - maybe I could add a check to see what files have changed after checkout and then short circuit the rest of the script when code files haven't changed.
There was a problem hiding this comment.
@inodb based on what I've discovered and communicated to you during our conversation this morning, I'm going to drop the effort to optimize the current configuration. I'll merge this PR once you give the approval. cc @pvannierop
n1zea144
force-pushed
the
fix-3850
branch
17 times, most recently
from
948a1d9
to
c6419ad
Compare
n1zea144
force-pushed
the
fix-3850
branch
8 times, most recently
from
1b07a6c
to
07e93c1
Compare
|
Small note: we should prolly update this once this one is fixed: #6844 |
Fix #3850