MirOil-for-Lomiri
-
Hi! I've started working on this... How should I work with this?
Create a branch and then send you PR? Or push changes directly into miroil branch?
Any way I don't have permission to push to a new branch....
-
Hi @erlend, thanks.
Please make PRs and I'll review as I get time. And also sync the work back to upstream MirOil branch.
I can also add you to the MirOil-for-Lomiri project to make that a little easier (need your github ID for that).
I do want to do an update about this work as:
- it is now possible to use a 20.04 base using the http://repo2.ubports.com/ repository; and,
- QtMir has migrated to gitlab (but I still need to sync with the right branch)
-
Okey... Shall I wait until you have updated it to 20.04? or can I start now?
What do you think?Github id is: 58894514 (erlend-g)
-
@erlend don't wait for me.
-
@alan_g Okey
-
Hi!
I still haven't got write permission to qtmir... Need it to create a branch... I tried forking the project but that does not work since i have already forked ubports/qtmir. I can't fork it more that once.
Can you fix that?
-
-
@alan_g Yes,Works.. Thank You!
-
I'm trying to find a good way to solve qtmir tests. For the test to work they need a lot of mocks that are based on internal mir stuff. And since they are used directly into mir it is not possible to build wrappers.
Include file Object test file Defined in mir/main_loop.h mir::MainLoop tests/framework/mock_main_loop.h src/include/server/mir/main_loop.h mir/scene/prompt_session.h mir::scene::PromptSession tests/framework/mock_prompt_session.h src/include/server/mir/scene/prompt_session.h mir/scene/surface.h> mir::scene::Surface tests/framework/mock_surface.h src/include/server/mir/scene/surface.h mir/shell/persistent_surface_store.h mir::shell::PersistentSurfaceStore
tests/framework/mock_persistent_surface_store.h src/include/server/mir/shell/persistent_surface_store.h mir/scene/session.h mir::scene::Session tests/framework/mock_mir_session.h src/include/server/mir/scene/session.h So the best solution (or least bad) that I have come up with is...
to move all those mocks to miroil and make them accessible through a MockFactory... Something like:class MockFactory { auto get_mock_surface() -> std::shared_ptr<mir::scene::Surface>; auto get_mock_main_loop() -> std::shared_ptr<mir::MainLoop>; auto get_mock_prompt_session() -> std::shared_ptr<mir::scene::PromptSession>; auto get_mock_persistent_surface_store() -> std::shared_ptr<mir::shell::PersistentSurfaceStore>; auto get_mock_session() -> std::shared_ptr<mir::scene::Session>; std::shared_ptr<mir::scene::Surface> surfaces; std::shared_ptr<mir::MainLoop> main_loops; std::shared_ptr<mir::scene::PromptSession> prompt_sessions; std::shared_ptr<mir::shell::PersistentSurfaceStore> persistent_surface_stores; std::shared_ptr<mir::scene::Session> sessions; };
The problem is that qtmir does not know the destructor of this objects, to they will have to be deleted inside miroil.
Therefore I have added a reference in MockFactor to the objects. Which are deleted by MockFactory. This will only work if MockFactory is the last object to be deleted, so we have to make sure it is.So do you guys see any better solutions than this?
-
@erlend thanks for bringing this up.
I don't have the headspace right now to look at the problematic tests. But, for background, Mir used to publish a test helpers library containing various stubs, mocked objects and test fixtures. That got unpublished along with the mirclient stuff that it depended heavily on. (It is on my "tech debt" list to reinstate something appropriate to the current APIs.)
I think a separate "testing support" library is a better place to support mocks than libmiroil.
Another possibility would be that at least some of these tests belong as tests of miroil (i.e. in the Mir project).
I will have a closer look, but not sure when I'll have time.
-
@alan_g Okey good, I'll start by upstream what is left, and then return to the test after that.
-
@erlend I've had a first look, and three of those test doubles are unused in the current codebase. There's a PR dropping them for your review.
The others are used in a range of tests I've yet to work through.