Remove local dev banner when doing e2e tests #3256
Conversation
There was a problem hiding this comment.
Just one question about this. Overall seems cleaner using css, so that's good. I just recall there was one test that checks whether the devmode banner is shown. This tests whether the frontend is actually loaded inside the backend project. It would be good to continue to be able to confirm this. Otherwise we might not be certain that what we are testing is actually the frontend in the PR. Could be done in another way then using the banner I guess
|
@inodb i think we could write that test to directly check the frontendConfig.frontendUrl variable |
|
@zhx828 based on screenshots it doesn't look like this worked. i think because .local-dev-banner is not actually applied to div. unfortunately that div is created in load_frontend.js of backend repo. Annoying. We might as well just move that to frontend. I mean the creation of that div. |
|
@alisman I added the class at the backend. We need to merge the backend to master and deploy before merging the frontend repo. cBioPortal/cbioportal#7573 |
|
@alisman checking frontendConfig.frontendUrl sounds good |
| it('window.frontendConfig.frontendUrl should point to localhost 3000 when testing', function() { | ||
| // We no longer check whether the dev mode banner exits. | ||
| // The banner is hidden in e2etests.scss | ||
| assert.equal( | ||
| browser.execute(function() { | ||
| return window.frontendConfig.frontendUrl; | ||
| }).value, | ||
| '//localhost:3000/' | ||
| ); |
Update the local dev check Signed-off-by: Hongxin Zhang <hongxin@cbio.mskcc.org>
|
Three failing tests are not related to this pr. |
This is a part of fixes for cBioPortal/cbioportal#7561