Allow tests to set the AppInfo to type HOST with a specific app id #1309
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
whot
approved these changes
Mar 18, 2024
swick
force-pushed
the
wip/test-app-info
branch
from
09265d2
to
f891634
Compare
|
Fixed :) |
|
I originally thought that the test suite was borked, but actually I think the tests are up to something. Need to figure out why this PR is crashing some of the test runs. |
|
@swick maybe try to rebase and force-push to retrigger the CI. I had issues in libwacom with ubuntu 22.04 and clang+asan for a while, that resolved itself last week and now it's all hunky and dory again. Could be the same issue here. |
The only use of the cunescape() functions is in the xdp-utils sources so let's couple those two together.
Tests can run in diverse environments, some of which would get picked up by our code that tries to find the app id. For tests we need predictable behavior. Introduce a new environment variable which can be set to an app id from which a XdpAppInfo of type HOST will be used for all connections.
Instead of querying x-d-p about the app id, we force the app id that x-d-p uses for all client connections.
Instead of querying x-d-p about the app id, we force the app id that x-d-p uses for all client connections. The `app_id` fixture and PortalMock allow to override this value. The `app_id` fixture can be used in tests to access the app_id that x-d-p will see.
|
Thanks, that seems to have worked. |
GeorgesStavracas
approved these changes
Apr 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This inverts the problem that #1279 and #1280 are trying to solve by passing in the app id that will be used for all client connections instead of querying for the app id that x-d-p found.
This is in particular useful because it removes the dependency on
xdp_get_app_info_from_pidwhich I'm removing in #1281