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

Add a CI tester for running libfwupd with QT threads #2640

Merged
merged 1 commit into from
Dec 7, 2020
Merged

Conversation

superm1
Copy link
Member

@superm1 superm1 commented Dec 3, 2020

This only runs on Arch to avoid giving the extra dependencies to all
the distro CI builds.

Type of pull request:

@superm1
Copy link
Member Author

superm1 commented Dec 3, 2020

@aleixpol your tester didn't have a license. Are you OK with it being licensed LGPL2 and included?

@superm1 superm1 marked this pull request as draft December 3, 2020 15:43
Copy link

@aleixpol aleixpol left a comment

Choose a reason for hiding this comment

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

I'm happy to have my code included in LGPL2, thanks!

meson_options.txt Outdated Show resolved Hide resolved
This only runs on Arch to avoid giving the extra dependencies to all
the distro CI builds.
@superm1 superm1 marked this pull request as ready for review December 3, 2020 17:03
@superm1 superm1 requested a review from hughsie December 3, 2020 17:03
@superm1
Copy link
Member Author

superm1 commented Dec 3, 2020

I would think it makes sense to rebase #2638 on this, and whatever the outcome that is done there lands in the tester cpp file.

@hughsie
Copy link
Member

hughsie commented Dec 3, 2020

The thing is; I don't know how QEventDispatcherGlib actually works. I don't know if the QT5 test case code is valid or not...

qt5concurrent = dependency('Qt5Concurrent')
glib2 = dependency('glib-2.0')
gio2 = dependency('gio-2.0')
fwupd = dependency('fwupd')
Copy link
Member

Choose a reason for hiding this comment

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

just to clarify; this is the installed fwupd version rather than the "just built" version, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it's explicitly run after install step.

Comment on lines +27 to +29
fw->setFuture(QtConcurrent::run([&] {
return fwupd_client_get_devices(client, cancellable, &error);
}));
Copy link
Member

Choose a reason for hiding this comment

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

if this hangs, how do we signify a CI failure? Is github actions going to kill our build after say 30 minutes? I'd much rather have a timer somewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a timeout set on the test in meson, so a hang should be killed by the test runner I would expect

Copy link
Member Author

Choose a reason for hiding this comment

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

But even if that doesn't happen for some reason yes the job will eventually timeout on GitHub too.

@superm1 superm1 merged commit d432161 into master Dec 7, 2020
@superm1 superm1 deleted the wip/threads branch December 7, 2020 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants