Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix two failing Python Tests on Ubuntu 20.04 #497

Merged
merged 3 commits into from
Oct 11, 2021
Merged

Fix two failing Python Tests on Ubuntu 20.04 #497

merged 3 commits into from
Oct 11, 2021

Conversation

whoenig
Copy link
Contributor

@whoenig whoenig commented Oct 6, 2021

  • Update test_videoOutput to use the correct Python version
  • Add missing information to docs

Fixes issue #496

* Update test_videoOutput to use the correct Python version
* Add missing information to docs

Fixes issue #496
@whoenig whoenig requested a review from jpreiss October 6, 2021 17:49
@whoenig
Copy link
Contributor Author

whoenig commented Oct 6, 2021

@jpreiss Any idea how to fix those failing tests? It looks to me like ffmpeg-python simply doesn't work with Python2 anymore.

@jpreiss
Copy link
Collaborator

jpreiss commented Oct 6, 2021

It seems like Python 2 CI is failing on the installation of python-ffmpeg from Pip.

I think it's reasonable to leave video out of the standard Crazyswarm dependency list. The pytest.mark.skip decorator for the video test currently checks "am I running on CI?", but we can replace it with one that checks "are ffmpeg and ffmpeg-python installed?"

This is what I did for the ROS import test - see 402f099.

@whoenig
Copy link
Contributor Author

whoenig commented Oct 6, 2021

I only added it to CI because it's missing in the documentation and the documentation is generated from CI ... Should we leave it out from the docs or have a separate manual line there?

@jpreiss
Copy link
Collaborator

jpreiss commented Oct 6, 2021

Yes, I was thinking of a separate line in the manual presented as an optional step. We never get issues/discussions about video output so I feel that most users do not use it.

CI:
* remove unused embedded gcc
* avoid using sudo for pip (this was also causing the documentation to
  suggest using sudo, which is bad practice)
Copy link
Collaborator

@jpreiss jpreiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ffmpeg-python accesses ffmpeg through the shell, so we might want to tell the user to sudo apt install -y ffmpeg as well.

@whoenig whoenig merged commit 27ef265 into master Oct 11, 2021
@whoenig whoenig deleted the issue_496 branch October 11, 2021 17:54
boomer319 referenced this pull request in boomer319/verrueckterschwarm2 Nov 25, 2023
* Fix two failing Python Tests on Ubuntu 20.04

* Update test_videoOutput to use the correct Python version
* Add missing information to docs
* List ffmpeg-python and ffmpeg as optional dependency

CI:
* remove unused embedded gcc
* avoid using sudo for pip (this was also causing the documentation to
  suggest using sudo, which is bad practice)

Fixes issue IMRCLab#496
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants