Run streamlit-folium tests as part of our CI #2117
Conversation
hansthen
changed the title
There was a problem hiding this comment.
Added a comment about how we install Streamlit-folium.
Something else, don't know if you already tried this. It's not critical for merging this, but could be nice. Maybe it's nice to verify this test works as intended by pushing a commit that should fail the test, then see if the pipeline fails. If it does, we know it works and can revert that commit. If we squash merge then the commit history on this branch doesn't really matter.
| - name: Test with pytest and retry flaky tests up to 3 times | ||
| run: | | ||
| cd streamlit_folium | ||
| pip install streamlit-folium |
There was a problem hiding this comment.
This installs the latest release from Pypi. While the tests we run are 'local' and from their main branch. Maybe that can lead to issues if their master branch has new tests that don't work on the latest release. Maybe it's safer to install the local version with pip install -e?
There was a problem hiding this comment.
Something else is I would put installing the package in its own step, now it's together with actually running the tests.
There was a problem hiding this comment.
Does it make a difference? I don't know much about best practices for the github CI/CD.
There was a problem hiding this comment.
This installs the latest release from Pypi. While the tests we run are 'local' and from their main branch. Maybe that can lead to issues if their master branch has new tests that don't work on the latest release. Maybe it's safer to install the local version with
pip install -e?
Yes good catch.
There was a problem hiding this comment.
Does it make a difference? I don't know much about best practices for the github CI/CD
As far as I know it doesn't make a functional difference, just makes it easier to read and follow.
Yes good idea. |
Closes #2112