Tests: compile into a single executable#2835
Conversation
leolost2605
left a comment
leolost2605
left a comment
There was a problem hiding this comment.
Makes sense to me! A few comments :)
Also maybe we should use the glib.test built in way to specify which tests to run? https://valadoc.org/glib-2.0/GLib.Test.init.html says we can just use -p to specify what to run. So we would just construct all testcases, put them in an array and then call test.init and test.run. This would also automatically give us more complete and finegrained control, e.g. to not only specify tests but also skip some, etc. We could also keep the name property if we maybe want to name them differently because we don't rely on the type name then.
|
@leolost2605 I don't think it's worth adding name field, at least until we really need it. Currently it's just an unneeded repetition in the code |
lenemter
force-pushed
the
lenemter/compile-tests-into-signle-executable
branch
2 times, most recently
from
06d7cec to
e3b3482
Compare
|
@leolost2605 for some reason this broke tests for mutter 50. I pushed a commit with a fix to this branch but let me know if you want to merge it separately and I'll create a new PR |
lenemter
force-pushed
the
lenemter/compile-tests-into-signle-executable
branch
from
e3b3482 to
103e9de
Compare
leolost2605
left a comment
leolost2605
left a comment
There was a problem hiding this comment.
Looks good, just a few more comments :)
@leolost2605 for some reason this broke tests for mutter 50. I pushed a commit with a fix to this branch but let me know if you want to merge it separately and I'll create a new PR
Yeah seems like we were doing remove_child (null) there so makes sense. If you clean up the commit history and add the fix as the first commit I think it's fine to merge with this
lenemter
force-pushed
the
lenemter/compile-tests-into-signle-executable
branch
2 times, most recently
from
2e1951d to
4b0eba5
Compare
|
@leolost2605 Done! |
lenemter
force-pushed
the
lenemter/compile-tests-into-signle-executable
branch
from
4b0eba5 to
e525f16
Compare
lenemter
force-pushed
the
lenemter/compile-tests-into-signle-executable
branch
from
e525f16 to
159a70f
Compare
leolost2605
left a comment
leolost2605
left a comment
There was a problem hiding this comment.
I've been thinking some more and I'm not sure anymore whether this is a good idea in its current form.
My main concern is that we loose any isolation of the test cases with this by implicitly allowing the possibility to run all at once. This wouldn't be as bad if we could just reconstruct the whole meta.context but since that doesn't work it poses a few problems:
- Resources that are accessed from multiple test cases might look different depending on if they are run together or alone and in which order. E.g. if one test case forgets to clean up the stage we might get problems there.
- We loose the possibility of customization of the environment for a specific test case. For example I think we probably want to provide the possibility for a test case to specify the type of the meta.plugin. For some a very simple mock is enough, other need a specific method mocked and maybe we want the actual windowmanagergala in some tests. Another thing is the monitor setup. In the light of recent multi monitor crashes we probably want a test case that tests multi monitor setups etc.
I came pretty far with --internal-vapi and a shared library but didn't quite finish it and will have a look again later this week. I think if we want to do single executable we will have to make sure that only a single test case can be run and don't want to have any static variables. A main file that decides the type, constructs it, and just calls run on the testcase would probably be better then.
Anyways just some of my thoughts on this, I'd be happy to hear other arguments and opinions :)
|
@leolost2605 everry test is still isolated though, no? Every test launches a new io.elementary.gala.tests and we set test(..., is_parallel: false) And customization of the test still can be done since the tests don't run in the same process and don't share mutter context afaict |
Like I said the approach itself makes sense but currently the isolation just happens to be there because we run the tests separately. I think we should really enforce it so no static meta.context, no possibility to run two tests at the same time already in the executable, etc. Like this (from the comment above):
|
2 participants
Closes #2833
Achieves the same performance boost as #2833, but doesn't introduce unnecessary #if conditions