Skip to content

Improve testing of the Rust code#2894

Merged
imobachgs merged 20 commits intoapi-v2from
better-testing
Nov 18, 2025
Merged

Improve testing of the Rust code#2894
imobachgs merged 20 commits intoapi-v2from
better-testing

Conversation

@imobachgs
Copy link
Copy Markdown
Contributor

@imobachgs imobachgs commented Nov 18, 2025

Problem

Testing our Rust code is kind of tricky. Many times, the code has to interact with the underlying system (systemd, NetworkManager, etc.) which makes hard even to test logic that it is not directly related.

Solution

  • Write a set of builders that allows to spawn services replacing some parts with testing versions. They allow to create services replacing the "adapters" (e.g., using a dummy network adapter).
  • Add a set of test_utils modules with some utilities to use during tests.
  • Expand the current tests, going from 21% to 26%.

Testing

  • Expanded the test coverage.

Follow-up

  • Create a test adapter for the software service.
  • Consider to do not inject any service by default in the builders. I would allow to replace any piece, but it is not needed by now.

MissingProposal,
}

/// Builds and spawns the l10n service.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have to say that I do not like much the change that basically make Builder to call spawn. It does not make much sense for me to say "Builder spawn service". if we go this way I would prefer to follow convention and have usual "new->with_*->build" flow for builder and for spawning having dedicated spawner or have it as method in that service like ServiceBuilder::new().build()?.spawn()?
This way it is inconsistent with rest of rust builder and is just confusing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see your point. Perhaps we need a better name instead of having a separate concept for spawning the service. For instance, Rust Command follows the builder pattern too, but it has better naming.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, you always spawn the service. After all, you communicate with it through messages. So I do not see the point of having separate build and spawn methods. Perhaps something like Launcher and launch. I do not like Spawner because the word does not exist :-) But I am open to other suggestions (perhaps something related to actors).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Launcher and launch sounds good to me. I just find confusing to have Builder and spawn. Alternative can be also "Starter"/"start" or "Bootstrap"/"start" or "Runner"/"run"

}

/// Spawns the manager service.
pub async fn spawn(self) -> Result<Handler<Service>, Error> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same note as above. In this case it is even more visible that it do building ( aka setting defaults for not specified stuff ) and then spawning together with service setup. Which I see as different tasks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For me it is similar to the standard Command struct.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In that case I would not call it Builder but Task. as you spawn tasks and not Builder


use crate::Service;

/// Spawns a testing manager service.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is there some magic to avoid compilation of this or should be here #cfg[test] ?

Copy link
Copy Markdown
Contributor Author

@imobachgs imobachgs Nov 18, 2025

Choose a reason for hiding this comment

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

No, it cannot be #cfg[test] because it cannot be used by other crates. The solution could be to put it behind a feature flag, but I decided to stop here (I think we have more urgent things to work on at this moment). But it could be a nice task for a Friday afternoon.

@imobachgs imobachgs merged commit e76ceb3 into api-v2 Nov 18, 2025
10 of 19 checks passed
@imobachgs imobachgs deleted the better-testing branch November 18, 2025 16:15
imobachgs added a commit that referenced this pull request Jan 10, 2026
Merge the new HTTP API. Each PR has been already reviewed, so it should
be safe to merge it.

* #1829
* #2508
* #2772
* #2826
* #2848
* #2860
* #2863
* #2866
* #2867
* #2869
* #2870
* #2871
* #2872
* #2873
* #2874
* #2875
* #2876
* #2877
* #2880
* #2881
* #2882
* #2884
* #2885
* #2886
* #2891
* #2892
* #2893
* #2894
* #2895
* #2896
* #2897
* #2898
* #2899
* #2900
* #2901
* #2902
* #2903
* #2904
* #2908
* #2909
* #2910
* #2912
* #2913
* #2914
* #2915
* #2916
* #2917
* #2918
* #2920
* #2921
* #2923
* #2924
* #2926
* #2928
* #2929
* #2930
* #2933
* #2934
* #2935
* #2936
* #2938
* #2939
* #2942
* #2943
* #2944
* #2945
* #2946
* #2947
* #2948
* #2950
* #2951
* #2952
* #2954
* #2955
* #2956
* #2957
* #2958
* #2959
* #2960
* #2961
* #2963
* #2964
* #2965
* #2967
* #2968
* #2969
* #2970
* #2971
* #2972
* #2974
* #2975
* #2977
* #2978
* #2980
* #2982
* #2983
* #2984
* #2988
* #2989
* #2991
* #2992
* #2993
* #2994
* #2995
* #2996
* #2997
* #2999
@imobachgs imobachgs mentioned this pull request Mar 17, 2026
imobachgs added a commit that referenced this pull request Mar 17, 2026
Prepare to release version 19.

* #1829
* #2508
* #2772
* #2818
* #2826
* #2848
* #2860
* #2863
* #2864
* #2866
* #2867
* #2869
* #2870
* #2871
* #2872
* #2873
* #2874
* #2875
* #2876
* #2877
* #2880
* #2881
* #2882
* #2884
* #2885
* #2886
* #2891
* #2892
* #2893
* #2894
* #2895
* #2896
* #2897
* #2898
* #2899
* #2900
* #2901
* #2902
* #2903
* #2904
* #2908
* #2909
* #2910
* #2912
* #2913
* #2914
* #2915
* #2916
* #2917
* #2918
* #2920
* #2921
* #2923
* #2924
* #2926
* #2928
* #2929
* #2930
* #2933
* #2934
* #2935
* #2936
* #2937
* #2938
* #2939
* #2942
* #2943
* #2944
* #2945
* #2946
* #2947
* #2948
* #2949
* #2950
* #2951
* #2952
* #2954
* #2955
* #2956
* #2957
* #2958
* #2959
* #2960
* #2961
* #2963
* #2964
* #2965
* #2967
* #2968
* #2969
* #2970
* #2971
* #2972
* #2974
* #2975
* #2977
* #2978
* #2980
* #2981
* #2982
* #2983
* #2984
* #2988
* #2989
* #2990
* #2991
* #2992
* #2993
* #2994
* #2995
* #2996
* #2997
* #2998
* #2999
* #3000
* #3001
* #3002
* #3004
* #3005
* #3006
* #3007
* #3008
* #3009
* #3011
* #3012
* #3013
* #3014
* #3015
* #3016
* #3018
* #3019
* #3020
* #3021
* #3022
* #3023
* #3024
* #3025
* #3026
* #3027
* #3028
* #3029
* #3030
* #3031
* #3033
* #3034
* #3035
* #3036
* #3037
* #3039
* #3040
* #3041
* #3042
* #3043
* #3044
* #3045
* #3046
* #3047
* #3048
* #3049
* #3050
* #3051
* #3052
* #3053
* #3054
* #3055
* #3056
* #3057
* #3058
* #3060
* #3061
* #3062
* #3063
* #3064
* #3065
* #3066
* #3067
* #3068
* #3069
* #3070
* #3071
* #3072
* #3073
* #3074
* #3075
* #3076
* #3077
* #3078
* #3079
* #3086
* #3087
* #3088
* #3089
* #3090
* #3091
* #3092
* #3093
* #3094
* #3095
* #3096
* #3097
* #3098
* #3099
* #3100
* #3101
* #3102
* #3103
* #3104
* #3105
* #3106
* #3107
* #3108
* #3109
* #3110
* #3112
* #3113
* #3114
* #3115
* #3116
* #3117
* #3118
* #3119
* #3120
* #3122
* #3123
* #3124
* #3127
* #3128
* #3129
* #3130
* #3131
* #3133
* #3134
* #3135
* #3136
* #3137
* #3138
* #3139
* #3140
* #3141
* #3142
* #3143
* #3144
* #3145
* #3146
* #3147
* #3148
* #3149
* #3150
* #3151
* #3152
* #3153
* #3154
* #3155
* #3157
* #3158
* #3159
* #3160
* #3161
* #3162
* #3163
* #3164
* #3165
* #3166
* #3167
* #3168
* #3169
* #3170
* #3174
* #3175
* #3176
* #3177
* #3178
* #3179
* #3181
* #3182
* #3184
* #3185
* #3186
* #3188
* #3189
* #3190
* #3191
* #3192
* #3194
* #3195
* #3196
* #3197
* #3198
* #3199
* #3200
* #3201
* #3202
* #3203
* #3205
* #3206
* #3208
* #3209
* #3210
* #3213
* #3214
* #3215
* #3216
* #3217
* #3218
* #3219
* #3220
* #3222
* #3223
* #3224
* #3225
* #3226
* #3227
* #3228
* #3229
* #3230
* #3231
* #3232
* #3233
* #3234
* #3235
* #3236
* #3237
* #3238
* #3239
* #3240
* #3241
* #3242
* #3243
* #3244
* #3246
* #3247
* #3248
* #3250
* #3251
* #3252
* #3253
* #3254
* #3255
* #3256
* #3257
* #3258
* #3259
* #3260
* #3261
* #3262
* #3263
* #3265
* #3266
* #3267
* #3268
* #3269
* #3270
* #3271
* #3272
* #3273
* #3274
* #3275
* #3276
* #3277
* #3278
* #3279
* #3280
* #3281
* #3282
* #3283
* #3284
* #3285
* #3286
* #3287
* #3288
* #3289
* #3290
* #3291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants