Run browser tests in safari #684
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #684 +/- ##
=======================================
Coverage 79.64% 79.64%
=======================================
Files 9 9
Lines 3930 3930
=======================================
Hits 3130 3130
Misses 800 800 🚀 New features to boost your workflow:
|
mcbarton
force-pushed
the
enable-safari-testing
branch
from
b7e3db9 to
27adc2d
Compare
mcbarton
changed the title
|
This PR is ready for review. Safari has no headless mode unlike other browsers, so doing automated testing with it is done a little differently to other browsers. I run emrun with the no_browser option, and then use safari driver with selenium to run the webpage we want. |
|
I have also enabled running the browser tests in Safari in xeus-cpp here compiler-research/xeus-cpp#368 . This PR is also ready for review. |
mcbarton
force-pushed
the
enable-safari-testing
branch
2 times, most recently
from
8b0a90f to
2b980f9
Compare
|
@anutosh491 @vgvassilev pinging for review. Also please the review the PR adding Safari browser testing in xeus-cpp. We can detect failures just like for other browsers. The only difference between this and other browsers, is we use safari driver and selenium to open the needed webpage, since Safari doesn't have a headless browser mode. |
mcbarton
force-pushed
the
enable-safari-testing
branch
from
2b980f9 to
b836470
Compare
unittests/CppInterOp/CMakeLists.txt
Outdated
| @@ -1,3 +1,4 @@ | |||
| file(COPY ${CMAKE_SOURCE_DIR}/scripts/browser_tests_safari.py DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) | |||
There was a problem hiding this comment.
Why do we need to copy this in the build folder?
There was a problem hiding this comment.
Just so that after I have run the emrun command I don't give a relative path to the python script. Technically you don't need it, but it makes the python command look tidier.
There was a problem hiding this comment.
I'd prefer to not copy it since it can cause invalidation issues in development areas if one gets edited.
There was a problem hiding this comment.
The PR has now been updated to use the relative path instead of copying the file. I think I got the relative path correct, but will know for certain once the ci finishes running.
There was a problem hiding this comment.
We probably have some github action magic to give the full path to the source dir.
There was a problem hiding this comment.
We could make it nicer in the ci using the environment variable for the github workspace, but feels like its best to use relative path there too, given that is what a user will have to do when following the documentation
There was a problem hiding this comment.
I got the relative path correct, but forgot to modify the static library Emscripten tests to use thee relative path in the ci. This has been fixed now, and the ci should pass this time.
mcbarton
force-pushed
the
enable-safari-testing
branch
from
b836470 to
608489e
Compare
Description
Please include a summary of changes, motivation and context for this PR.
Fixes # (issue)
Type of change
Please tick all options which are relevant.
Testing
Please describe the test(s) that you added and ran to verify your changes.
Checklist