-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
MAVSDK testing improvements #14296
MAVSDK testing improvements #14296
Conversation
37423b7
to
5c8c922
Compare
Nice! What is needed to get this from WIP to ready for review? |
@LorenzMeier The output is not like I want it yet. There should be minimal output unless something fails and if so, all the information is required to see what is going on. |
e9a39a1
to
07e9aa1
Compare
@LorenzMeier ready for review and merging (given CI passes). |
78e1c05
to
d8defb6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice structure, looks good to me. I have one question regarding the idle item.
d8defb6
to
9e25910
Compare
Merging depends on #14134. |
Without the library installed, we can't build and run the tests. Silently ignoring this just leads to confusion.
This is required because some tests don't work at more than 1x.
It doesn't actually seem to work.
We can put them back once it's supported.
Otherwise, the script will get a tangled mess.
This way tests can be run at different locations.
Otherwise this raises a KeyError.
202fae4
to
3aaa432
Compare
Resolved, ready to be merged. |
This includes MAVSDK testing improvements.
The changes include:
And this is what it looks like now: