Skip to content

Commit c6805c8

Browse files
committed
chore: remove duplicate test, simplify comments
1 parent 8f96ed8 commit c6805c8

8 files changed

+57
-250
lines changed

tests/auto_detection_test.ml

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,9 @@ open Test_helpers
22
open Auto_detection
33
open Alcotest
44

5-
(** Test GitLab info detection - should fallback to default domain if not found
6-
*)
75
let test_auto_detect_gitlab_info () =
86
(* Test with real GitLab API if credentials are available, otherwise use mock *)
9-
let bot_info =
10-
match create_real_bot_info () with
11-
| Some info ->
12-
info (* Use real credentials if available *)
13-
| None ->
14-
create_mock_bot_info () (* Fallback to mock *)
15-
in
7+
let bot_info = get_bot_info () in
168
let owner = "test-org" in
179
let repo = "test-repo" in
1810
let result =
@@ -30,12 +22,12 @@ let test_auto_detect_gitlab_info () =
3022

3123
(** Test org/team detection with real API or graceful skip *)
3224
let test_auto_detect_org_team () =
33-
(* Test with real GitHub API if credentials are available *)
34-
match create_real_bot_info () with
25+
let bot_info = get_bot_info () in
26+
(* Skip test if no real credentials - this is expected in CI without secrets *)
27+
match bot_info.github_install_token with
3528
| None ->
36-
(* Skip test if no credentials - this is expected in CI without secrets *)
3729
Alcotest.skip ()
38-
| Some bot_info -> (
30+
| Some _ -> (
3931
(* Use a real public repository for testing that is known to exist *)
4032
let owner = "ocaml" in
4133
let repo = "opam" in
@@ -63,12 +55,12 @@ let test_auto_detect_org_team () =
6355

6456
(** Test complete auto-detection with caching *)
6557
let test_auto_detect_from_apis () =
66-
(* Test with real GitHub API if credentials are available *)
67-
match create_real_bot_info () with
58+
let bot_info = get_bot_info () in
59+
(* Skip test if no real credentials *)
60+
match bot_info.github_install_token with
6861
| None ->
69-
(* Skip test if no credentials - this is expected in CI without secrets *)
7062
Alcotest.skip ()
71-
| Some bot_info ->
63+
| Some _ ->
7264
(* Use a real public repository for testing *)
7365
let owner = "ocaml" in
7466
let repo = "opam" in
@@ -102,10 +94,12 @@ let test_auto_detect_from_apis () =
10294

10395
(** Test that auto_detect_from_apis returns a complete Repo_config.t *)
10496
let test_auto_detect_from_apis_completeness () =
105-
match create_real_bot_info () with
97+
let bot_info = get_bot_info () in
98+
(* Skip test if no real credentials *)
99+
match bot_info.github_install_token with
106100
| None ->
107101
Alcotest.skip ()
108-
| Some bot_info ->
102+
| Some _ ->
109103
let owner = "ocaml" in
110104
let repo = "opam" in
111105
Cache.clear_all () ;
@@ -124,8 +118,6 @@ let test_auto_detect_from_apis_completeness () =
124118
check bool "org_name is Some" (Option.is_some result.org_name) true ;
125119
(* team_name should be Some *)
126120
check bool "team_name is Some" (Option.is_some result.team_name) true ;
127-
(* minimizer_url may be Some (if BOT_MINIMIZER_URL env var is set) or None (generic default) *)
128-
(* This is acceptable - minimizer_url is optional and can be configured per-repo *)
129121
(* ci_config should be Some (from defaults) *)
130122
check bool "ci_config is Some" (Option.is_some result.ci_config) true
131123

tests/config_resolver_test.ml

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,7 @@ open Repo_config
22
open Alcotest
33
open Test_helpers
44

5-
let bot_info =
6-
match create_real_bot_info () with
7-
| Some info ->
8-
info
9-
| None ->
10-
create_mock_bot_info ()
5+
let bot_info = get_bot_info ()
116

127
let test_merge_priority_explicit_overrides () =
138
let explicit_config =
@@ -70,7 +65,6 @@ let test_merge_priority_api_fills_gaps () =
7065
check (option string) "api team_name" result.team_name (Some "maintainers")
7166

7267
let test_merge_priority_defaults_fallback () =
73-
(* Skip test if no real credentials - API calls require installation token *)
7468
( match bot_info.github_install_token with
7569
| None ->
7670
Alcotest.skip ()

tests/default_config_test.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,5 @@ let () =
4848
[ ( "defaults"
4949
, [test_case "defaults for any repo" `Quick test_defaults_for_any_repo] )
5050
; ( "no_hardcoded_patterns"
51-
, [ test_case "defaults for any repo" `Quick
51+
, [ test_case "no hardcoded patterns" `Quick
5252
test_defaults_no_hardcoded_patterns ] ) ]

tests/generic_bot_demo_test.ml

Lines changed: 31 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -6,26 +6,16 @@ open Alcotest
66
open Default
77

88
(**
9-
* Demonstration tests showing how the generic bot configuration system works.
9+
* Tests demonstrating the generic bot configuration system.
1010
*
11-
* Key concepts demonstrated:
12-
* 1. Rocq is just a configured instance - no special code, just configuration
13-
* 2. Any repository can be configured with minimal effort (just github = "owner/repo")
14-
* 3. 3-tier configuration resolution: Explicit > Auto-Detection > Defaults
15-
* 4. Flexible configuration levels: full, partial, or minimal
16-
*
17-
* Note: These are HIGH-LEVEL demonstrations. Detailed tests are in:
18-
* - repo_config_test.ml (parsing)
19-
* - repo_config_integration_test.ml (multi-repo, file loading)
20-
* - config_resolver_test.ml (resolution priority)
21-
* - auto_detection_test.ml (API auto-detection)
22-
* - default_config_test.ml (defaults)
11+
* Key concepts:
12+
* - Rocq is just a repo with explicit config, same as any other repo
13+
* - Adding a repo requires only: github = "owner/repo"
14+
* - Config resolution: explicit > auto-detection > defaults
15+
* - Supports full, partial, or minimal configuration
2316
*)
2417

25-
(** Demo 1: Rocq is a fully configured instance - no special code *)
2618
let test_rocq_is_configured_instance () =
27-
(* Rocq is NOT hardcoded in the bot. It's just a repository with explicit configuration.
28-
This shows that rocq-prover/rocq uses explicit config for features it needs. *)
2919
let toml_str =
3020
{|
3121
[repositories.rocq]
@@ -46,7 +36,7 @@ custom_job_status = true
4636
Option.value_exn
4737
(get_repo_config_opt ~owner:"rocq-prover" ~repo:"rocq" table)
4838
in
49-
(* Rocq explicitly configures features it needs *)
39+
(* Verify rocq's explicit configuration *)
5040
check bool "rocq configures custom GitLab domain (not default)"
5141
(Option.is_some rocq_config.gitlab_domain)
5242
true ;
@@ -60,14 +50,12 @@ custom_job_status = true
6050
| None ->
6151
false )
6252
true ;
63-
(* Key point: Same configuration system as any other repo *)
6453
check string "rocq uses standard config format" rocq_config.github_owner
6554
"rocq-prover"
6655

67-
(** Demo 2: Any repository works with just one line of config *)
6856
let test_minimal_repo_works_immediately () =
69-
(* Show how easy it is to add ANY repository to the bot.
70-
Just specify github = "owner/repo" and everything else is handled automatically. *)
57+
(* Adding a repo requires only github = "owner/repo". The bot handles the rest.
58+
This test shows the parsing, defaults, and resolution steps. *)
7159
let toml_str = {|
7260
[repositories.my-new-repo]
7361
github = "my-org/my-repo"
@@ -77,10 +65,10 @@ github = "my-org/my-repo"
7765
let config =
7866
Option.value_exn (get_repo_config_opt ~owner:"my-org" ~repo:"my-repo" table)
7967
in
80-
(* Step 1: Parsing - only explicit values are present *)
68+
(* Step 1: Parsing extracts only explicit TOML values *)
8169
check string "explicit github_owner from TOML" config.github_owner "my-org" ;
8270
check string "explicit github_repo from TOML" config.github_repo "my-repo" ;
83-
(* Missing fields are None at parse time - this is expected *)
71+
(* Missing fields are None after parsing *)
8472
check bool "gitlab_domain not in TOML, so None after parsing"
8573
(Option.is_none config.gitlab_domain)
8674
true ;
@@ -91,8 +79,8 @@ github = "my-org/my-repo"
9179
check (option string) "default org_name" defaults.org_name (Some "my-org") ;
9280
check (option string) "default team_name" defaults.team_name
9381
(Some "maintainers") ;
94-
(* Step 3: When used in the bot, config_resolver applies defaults *)
95-
(* Create mock bot_info *)
82+
(* Step 3: Config resolver applies defaults at runtime *)
83+
(* Create mock bot_info for resolution *)
9684
let gitlab_instances = Hashtbl.create (module String) in
9785
Hashtbl.set gitlab_instances ~key:"gitlab.com" ~data:("test-bot", "test-token") ;
9886
let bot_info =
@@ -108,7 +96,7 @@ github = "my-org/my-repo"
10896
Lwt_main.run
10997
(Config_resolver.resolve_repo_config ~bot_info ~explicit_config:config)
11098
in
111-
(* After resolution: explicit values preserved, defaults applied to missing fields *)
99+
(* After resolution: explicit values preserved, defaults applied *)
112100
check string "explicit values preserved" resolved.github_owner "my-org" ;
113101
check (option string) "defaults applied: gitlab_domain" resolved.gitlab_domain
114102
(Some "gitlab.com") ;
@@ -120,15 +108,12 @@ github = "my-org/my-repo"
120108
(Option.is_some resolved.ci_config)
121109
true ;
122110
check bool "defaults applied: labels" (Option.is_some resolved.labels) true ;
123-
(* Conclusion: Repository works immediately with just 1 line of config! *)
124111
()
125112

126-
(** Demo 3: The 3-tier configuration system *)
127113
let test_three_tier_config_system () =
128-
(* Demonstrate: Explicit > Auto-Detection > Defaults
129-
This shows how the bot intelligently fills in missing configuration. *)
130-
131-
(* Example 1: Minimal config triggers auto-detection *)
114+
(* Config resolution priority: explicit values first, then auto-detection from APIs,
115+
then defaults. This test shows when each path is used. *)
116+
(* Case 1: Minimal config triggers auto-detection *)
132117
let minimal_toml =
133118
{|
134119
[repositories.minimal]
@@ -141,12 +126,12 @@ github = "test-org/test-repo"
141126
Option.value_exn
142127
(get_repo_config_opt ~owner:"test-org" ~repo:"test-repo" table)
143128
in
144-
(* Auto-detection will run because gitlab_domain and org_name are missing *)
129+
(* Missing fields trigger auto-detection *)
145130
check bool "missing fields trigger auto-detection"
146131
( Option.is_none minimal_config.gitlab_domain
147132
&& Option.is_none minimal_config.org_name )
148133
true ;
149-
(* Example 2: Explicit config skips auto-detection *)
134+
(* Case 2: Explicit config skips auto-detection *)
150135
let explicit_toml =
151136
{|
152137
[repositories.explicit]
@@ -161,18 +146,16 @@ org_name = "test-org"
161146
Option.value_exn
162147
(get_repo_config_opt ~owner:"test-org" ~repo:"test-repo" table2)
163148
in
164-
(* Auto-detection skipped because key fields are present *)
149+
(* Explicit fields skip auto-detection *)
165150
check bool "explicit fields skip auto-detection"
166151
( Option.is_some explicit_config.gitlab_domain
167152
&& Option.is_some explicit_config.org_name )
168153
true ;
169-
(* Conclusion: Bot is smart - auto-detects when needed, uses explicit config when available *)
170154
()
171155

172-
(** Demo 4: Partial configuration - override only what you need *)
173156
let test_partial_config_flexibility () =
174-
(* Show that you can override specific defaults while keeping others.
175-
This demonstrates flexibility: configure as much or as little as you want. *)
157+
(* We can override specific defaults while keeping the rest. This allows
158+
customizing only what's needed without repeating all config. *)
176159
let toml_str =
177160
{|
178161
[repositories.custom]
@@ -192,17 +175,14 @@ team_name = "developers"
192175
config.gitlab_domain (Some "gitlab.example.com") ;
193176
check (option string) "explicit team_name overrides default" config.team_name
194177
(Some "developers") ;
195-
(* Missing values use defaults *)
178+
(* Unspecified fields use defaults *)
196179
let defaults = get_defaults ~owner:"custom-org" ~repo:"custom-repo" in
197180
check (option string) "org_name uses default" defaults.org_name
198181
(Some "custom-org") ;
199-
(* Conclusion: Mix and match explicit config with defaults as needed *)
200182
()
201183

202-
(** Demo 5: Multiple repositories with different config levels *)
184+
(** Tests multiple repos with different config levels in the same file *)
203185
let test_multiple_repos_coexist () =
204-
(* Show that repos can have different config levels and work independently.
205-
This demonstrates the bot's flexibility for different use cases. *)
206186
let toml_str =
207187
{|
208188
[repositories.full-featured]
@@ -219,14 +199,14 @@ github = "org2/repo2"
219199
in
220200
let toml_data = Utils.toml_of_string toml_str in
221201
let table = repo_config_table toml_data in
222-
(* Both repositories exist in the same config *)
202+
(* Both repos exist in the same config *)
223203
check bool "full-featured repo configured"
224204
(has_repo_config ~owner:"org1" ~repo:"repo1" table)
225205
true ;
226206
check bool "basic repo configured"
227207
(has_repo_config ~owner:"org2" ~repo:"repo2" table)
228208
true ;
229-
(* They have different config levels *)
209+
(* Verify they have different config levels *)
230210
let full_config =
231211
Option.value_exn (get_repo_config_opt ~owner:"org1" ~repo:"repo1" table)
232212
in
@@ -239,13 +219,10 @@ github = "org2/repo2"
239219
check bool "basic has no project_number (backport disabled)"
240220
(Option.is_none basic_config.github_project_number)
241221
true ;
242-
(* Conclusion: Different repos can have different feature sets *)
243222
()
244223

245-
(** Demo 6: Rocq vs generic repo - both use the same system *)
224+
(** Compares rocq and a generic repo to show they use the same config system *)
246225
let test_rocq_vs_generic_same_system () =
247-
(* Demonstrate that rocq is NOT special - it just has more explicit configuration.
248-
Any other repo could have the same features by adding the same config. *)
249226
let toml_str =
250227
{|
251228
[repositories.rocq]
@@ -266,59 +243,27 @@ github = "ocaml/opam"
266243
let opam_config =
267244
Option.value_exn (get_repo_config_opt ~owner:"ocaml" ~repo:"opam" table)
268245
in
269-
(* Both use the SAME configuration system *)
246+
(* Both use the same config system *)
270247
check string "rocq uses standard config" rocq_config.github_owner
271248
"rocq-prover" ;
272249
check string "opam uses standard config" opam_config.github_owner "ocaml" ;
273-
(* Difference is only in CONFIGURATION, not code *)
274250
check bool "rocq has explicit gitlab_domain"
275251
(Option.is_some rocq_config.gitlab_domain)
276252
true ;
277253
check bool "opam uses default gitlab_domain"
278254
(Option.is_none opam_config.gitlab_domain)
279255
true ;
280-
(* But both have access to defaults *)
256+
(* Both can access the same defaults *)
281257
let rocq_defaults = get_defaults ~owner:"rocq-prover" ~repo:"rocq" in
282258
let opam_defaults = get_defaults ~owner:"ocaml" ~repo:"opam" in
283259
check bool "both can use defaults"
284260
( Option.is_some rocq_defaults.gitlab_domain
285261
&& Option.is_some opam_defaults.gitlab_domain )
286262
true ;
287-
(* Conclusion: Rocq is just a configured instance, not special code *)
288-
()
289-
290-
(** Demo 7: Configuration is easy - just one line for basic setup *)
291-
let test_config_ease_of_use () =
292-
(* Show how simple it is to add a new repository to the bot.
293-
This is the key selling point: minimal configuration required. *)
294-
let minimal_toml =
295-
{|
296-
[repositories.new-repo]
297-
github = "new-org/new-repo"
298-
|}
299-
in
300-
let toml_data = Utils.toml_of_string minimal_toml in
301-
let table = repo_config_table toml_data in
302-
(* Just 1 line and the repo is ready to use *)
303-
check bool "repo configured with 1 line"
304-
(has_repo_config ~owner:"new-org" ~repo:"new-repo" table)
305-
true ;
306-
(* Defaults fill in everything else *)
307-
let defaults = get_defaults ~owner:"new-org" ~repo:"new-repo" in
308-
check bool "defaults provide gitlab_domain"
309-
(Option.is_some defaults.gitlab_domain)
310-
true ;
311-
check bool "defaults provide CI config"
312-
(Option.is_some defaults.ci_config)
313-
true ;
314-
check bool "defaults provide labels" (Option.is_some defaults.labels) true ;
315-
(* Conclusion: Bot is very easy to configure for any repository *)
316263
()
317264

318-
(** Demo 8: Complete configuration coverage - all available options *)
265+
(** Tests all available configuration options - serves as a reference *)
319266
let test_complete_config_coverage () =
320-
(* This test demonstrates ALL available configuration options.
321-
It serves as documentation for what can be configured. *)
322267
let toml_str =
323268
{|
324269
[repositories.complete]
@@ -376,12 +321,11 @@ ml_api_path = "path/to/ml-api.html"
376321
check (option string) "team_name" config.team_name (Some "maintainers") ;
377322
check (option string) "minimizer_url" config.minimizer_url
378323
(Some "https://custom-minimizer.com") ;
379-
(* Verify nested configs *)
324+
(* Verify nested config sections *)
380325
check bool "ci_config present" (Option.is_some config.ci_config) true ;
381326
check bool "labels present" (Option.is_some config.labels) true ;
382327
check bool "jobs present" (Option.is_some config.jobs) true ;
383328
check bool "documentation present" (Option.is_some config.documentation) true ;
384-
(* Conclusion: All fields are available for configuration *)
385329
()
386330

387331
let () =
@@ -401,8 +345,6 @@ let () =
401345
; ( "comparison"
402346
, [ test_case "rocq vs generic same system" `Quick
403347
test_rocq_vs_generic_same_system ] )
404-
; ( "ease_of_use"
405-
, [test_case "config ease of use" `Quick test_config_ease_of_use] )
406348
; ( "documentation"
407349
, [ test_case "complete config coverage" `Quick
408350
test_complete_config_coverage ] ) ]

0 commit comments

Comments
 (0)