cbioportal-frontend
cbioportal-frontend copied to clipboard
e2eTests - Screenshots/Reference: fixed files with invalid paths preventing repo checkout on Windows environment
BACKGROUND
Without this commit, it is not possible to checkout the repo in a Windows environment (and possibly others) There are 5 screenshots under e2etests that have invalid path characters in the name. Git on Windows will simply abort a checkout in this scenario. Some of the files have a '>' in the name which is good idea to avoid for command line work anyways.
DESCRIPTION
In this PR, 4 of the screenshots with invalid paths have been deleted. They already have corrected versions. One of the screenshots was renamed (the ":" to a "_" following the convention for the other files).
Deleted Files with other versions already existing:
end-to-end-test/local/screenshots/reference/shows_`value_>8.00`_in_figure_legend_and_indicates_sub-threshold_data_points_in_plot_element_chrome_1600x1000.pngend-to-end-test/local/screenshots/reference/when_option_deselected,_hides_`value_>8.00`_in_figure_legend_and_sub-threshold_data_point_indicators_in_waterfall_plot_element_chrome_1600x1000.pngend-to-end-test/local/screenshots/reference/when_option_deselected,_hides_`value_>8.00`_in_figure_legend_and_sub-threshold_data_points_in_plot_element_chrome_1600x1000.png'end-to-end-test/remote/screenshots/reference/msk_impact_2017_query_stk11:homdel_mut_element_chrome_1600x1000.png
Renamed Files:
end-to-end-test/remote/screenshots/reference/download_tab_-_nsclc_tcga_broad_2016_for_query_egfr:_mut=t790m_amp_element_chrome_1600x1000.pngCHANGE: renamed the : to _ NEW NAME:download_tab_-_nsclc_tcga_broad_2016_for_query_egfr__mut=t790m_amp_element_chrome_1600x1000.png
ISSUE TRACKER
Fixes cBioPortal/cbioportal#10836
Tests
No tests have been added. Arguably, a pre-commit trigger could be added to check filenames for invalid path characters, or a test, but this is likely a rare scenario so a low priority task could be added.
Also, not sure where references to the screenshots used in e2etests are stored. At least the one reference to the renamed file may need to be updated.
Any screenshots or GIFs?
n/a
Notify reviewers
TBD
Deploy Preview for cbioportalfrontend ready!
| Name | Link |
|---|---|
| Latest commit | 2a07708c327e3e0f3a077e6cc6fad0f986e1d284 |
| Latest deploy log | https://app.netlify.com/sites/cbioportalfrontend/deploys/669051442851600008d653ca |
| Deploy Preview | https://deploy-preview-4927--cbioportalfrontend.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
I see the CI failed on e2e tests but these failures are the same as the master branch.
@pappde thanks for doing this. These filenames are autogenerated based on the describe/it labels in specs. So to fix this:
- change the labels in the spec files to get rid of the problem chars
- push and tests will run in CI env, which will create new reference screenshots that we can download and commit to repo (and delete old ones) Happy to help with this. Maybe you could just go change the labels
it('LABEL IS THIS STRING HERE', function(){...})
@alisman That was very helpful. I tracked down where the screenshot filenames are generated from the test name, and updated it to fix all invalid path characters. That commit has been added to this PR.
For the 4 screenshots above that are marked "Deleted Files with other versions already existing", I verified that the test names were already updated to reference the renamed file. So the old files (with invalid characters) were orphans, safely removed.
For the 5th screenshot above marked "Renamed Files", this new commit fixes it so the test name matches the corrected filename. There is no need to change any test names.
I verified this change doesn't impact any of the existing screenshot files.
@alisman Hi, I am still experiencing this issue on a Windows system, seems like this commit hasn't been merged to master yet. Do you have any plans regarding this?