Skip to content

Commit 00947fc

Browse files
committed
docs(agents): fix 5 review issues in testorg_plan.md
H-1: Change D-1 status-table dependency from Phase B to Phase C; the CMakeLists.txt example uses post-C-1 filenames, so D-1 must run after C-1, not immediately after Phase B. Also update the Phase D header to state this dependency explicitly. H-2: Move undirected_dfs.cpp row to its correct alphabetical position in the Step C-1 rename table (after tiernan_all_cycles, not between finish_edge_bug and graph). M-1: Rename A-3 section heading from 'SGB/LEDA' to 'SDB/LEDA' to match the actual Jamfile flag name (-sSDB=). M-2: Remove unused 'import path ;' from the B-1 concepts Jamfile example — no path.* rules are called in that sub-Jamfile. M-3: Add a Multi-variant tests block to the D-1 CMakeLists.txt snippet explaining that bgl_test() cannot handle graph_1..9 / property_iter_1..9, with an explicit add_executable / add_test pattern for those cases.
1 parent b0ef20d commit 00947fc

1 file changed

Lines changed: 101 additions & 47 deletions

File tree

.github/agents/testorg_plan.md

Lines changed: 101 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ rmdir test/concept_tests
142142
This is a directory move, not a file-by-file copy. After the move, remove the
143143
now-empty `test/concept_tests/` parent directory if it still exists.
144144

145-
**SGB/LEDA conditional compile files** (only compiled when `-sSDB=` or
145+
**SDB/LEDA conditional compile files** (only compiled when `-sSDB=` or
146146
`-sLEDA=` is set) — move as well:
147147

148148
| Source | Destination |
@@ -298,7 +298,6 @@ Create a minimal `Jamfile.v2` in each new subdirectory
298298
for `test/concepts/Jamfile.v2`:
299299

300300
```jam
301-
import path ;
302301
project : requirements <library>/boost/graph//boost_graph ;
303302
304303
alias concepts_tests :
@@ -435,65 +434,110 @@ be a separate PR.
435434

436435
### Step C-1 — Rename test files to `*_test.cpp`
437436

438-
Rename every runtime test file that does not already end in `_test.cpp` or
439-
`_cc.cpp`. Use the table below as a starting point, then run a final search for
440-
any remaining top-level `test/*.cpp` runtime tests that still use bare or
441-
hyphenated names before closing the step.
437+
Rename every current `[ run ... ]` target in `test/Jamfile.v2` that does not
438+
already end in `_test.cpp` or `_cc.cpp`, except for the `*2` variants deferred
439+
to Step C-3 and the explicit framework-file exception noted below. The table
440+
below was verified programmatically against the current Jamfile runtime
441+
inventory, normalized through the Phase A-3, A-4, and B-4 path moves that must
442+
already be in place before Phase C starts. If the Jamfile changes before this
443+
step lands, regenerate the inventory and refresh the table in the same commit.
442444

443445
| Current name | Proposed new name | Notes |
444446
|---|---|---|
445-
| `test/bfs.cpp` | `test/bfs_test.cpp` | |
446-
| `test/dfs.cpp` | `test/dfs_test.cpp` | |
447-
| `test/undirected_dfs.cpp` | `test/undirected_dfs_test.cpp` | |
448-
| `test/undirected_dfs_visitor.cpp` | `test/undirected_dfs_visitor_test.cpp` | |
449-
| `test/graph.cpp` | `test/graph_test.cpp` | Check for name collision with `graph_test.hpp`; rename header first (already done in A-2) |
450-
| `test/subgraph.cpp` | `test/subgraph_test.cpp` | |
451-
| `test/subgraph_add.cpp` | `test/subgraph_add_test.cpp` | |
452-
| `test/subgraph_bundled.cpp` | `test/subgraph_bundled_test.cpp` | |
453-
| `test/subgraph_props.cpp` | `test/subgraph_props_test.cpp` | |
454-
| `test/copy.cpp` | `test/copy_test.cpp` | compile-only |
455-
| `test/swap.cpp` | `test/swap_test.cpp` | compile-only |
456-
| `test/property_iter.cpp` | `test/property_iter_test.cpp` | compile-only |
457-
| `test/lvalue_pmap.cpp` | `test/lvalue_pmap_test.cpp` | compile-only |
458-
| `test/dimacs.cpp` | `test/dimacs_test.cpp` | compile-only |
459-
| `test/filtered_graph_properties_dijkstra.cpp` | `test/filtered_graph_properties_dijkstra_test.cpp` | compile-only |
447+
| `test/adj_list_edge_list_set.cpp` | `test/adj_list_edge_list_set_test.cpp` | |
448+
| `test/adj_list_loops.cpp` | `test/adj_list_loops_test.cpp` | |
460449
| `test/bellman-test.cpp` | `test/bellman_test.cpp` | fix hyphen too |
461-
| `test/johnson-test.cpp` | `test/johnson_test.cpp` | fix hyphen too |
462-
| `test/isomorphism.cpp` | `test/isomorphism_test.cpp` | |
463-
| `test/hawick_circuits.cpp` | `test/hawick_circuits_test.cpp` | |
464-
| `test/tiernan_all_cycles.cpp` | `test/tiernan_all_cycles_test.cpp` | |
450+
| `test/bfs.cpp` | `test/bfs_test.cpp` | |
451+
| `test/bidir_remove_edge.cpp` | `test/bidir_remove_edge_test.cpp` | |
465452
| `test/bron_kerbosch_all_cliques.cpp` | `test/bron_kerbosch_all_cliques_test.cpp` | |
453+
| `test/bundled_properties.cpp` | `test/bundled_properties_test.cpp` | |
466454
| `test/closeness_centrality.cpp` | `test/closeness_centrality_test.cpp` | |
467-
| `test/degree_centrality.cpp` | `test/degree_centrality_test.cpp` | |
468-
| `test/mean_geodesic.cpp` | `test/mean_geodesic_test.cpp` | |
469-
| `test/eccentricity.cpp` | `test/eccentricity_test.cpp` | |
470455
| `test/clustering_coefficient.cpp` | `test/clustering_coefficient_test.cpp` | |
471-
| `test/sequential_vertex_coloring.cpp` | `test/sequential_vertex_coloring_test.cpp` | |
456+
| `test/concepts/clustering/compile_louvain_graph_types.cpp` | `test/concepts/clustering/compile_louvain_graph_types_test.cpp` | path assumes Phase A-3 already landed |
457+
| `test/concepts/clustering/compile_louvain_quality_function.cpp` | `test/concepts/clustering/compile_louvain_quality_function_test.cpp` | path assumes Phase A-3 already landed |
472458
| `test/cuthill_mckee_ordering.cpp` | `test/cuthill_mckee_ordering_test.cpp` | |
473-
| `test/king_ordering.cpp` | `test/king_ordering_test.cpp` | |
474-
| `test/delete_edge.cpp` | `test/delete_edge_test.cpp` | |
475-
| `test/serialize.cpp` | `test/serialize_test.cpp` | |
476-
| `test/read_propmap.cpp` | `test/read_propmap_test.cpp` | |
477-
| `test/generator_test.cpp` | already correct | |
459+
| `test/cycle_ratio_tests.cpp` | `test/cycle_ratio_test.cpp` | drop redundant plural while normalizing |
478460
| `test/dag_longest_paths.cpp` | `test/dag_longest_paths_test.cpp` | |
479-
| `test/bundled_properties.cpp` | `test/bundled_properties_test.cpp` | |
480-
| `test/parallel_edges_loops_test.cpp` | already correct | |
481-
| `test/bidir_remove_edge.cpp` | `test/bidir_remove_edge_test.cpp` | |
482-
| `test/bidir_vec_remove_edge.cpp` | `test/bidir_vec_remove_edge_test.cpp` | |
461+
| `test/degree_centrality.cpp` | `test/degree_centrality_test.cpp` | |
462+
| `test/delete_edge.cpp` | `test/delete_edge_test.cpp` | |
463+
| `test/dfs.cpp` | `test/dfs_test.cpp` | |
464+
| `test/benchmarks/dijkstra_no_color_map_compare.cpp` | `test/benchmarks/dijkstra_no_color_map_compare_test.cpp` | path assumes Phase B-4 already landed |
465+
| `test/eccentricity.cpp` | `test/eccentricity_test.cpp` | |
466+
| `test/regressions/finish_edge_bug.cpp` | `test/regressions/finish_edge_bug_test.cpp` | path assumes Phase A-4 already landed |
467+
| `test/graph.cpp` | `test/graph_test.cpp` | Check for name collision with `graph_test.hpp`; rename header first (already done in A-2) |
468+
| `test/hawick_circuits.cpp` | `test/hawick_circuits_test.cpp` | |
483469
| `test/index_graph.cpp` | `test/index_graph_test.cpp` | |
470+
| `test/isomorphism.cpp` | `test/isomorphism_test.cpp` | |
471+
| `test/johnson-test.cpp` | `test/johnson_test.cpp` | fix hyphen too |
472+
| `test/king_ordering.cpp` | `test/king_ordering_test.cpp` | |
484473
| `test/labeled_graph.cpp` | `test/labeled_graph_test.cpp` | |
485-
| `test/named_vertices_test.cpp` | already correct | |
486-
| `test/filter_graph_vp_test.cpp` | already correct | |
487-
| `test/test_graphs.cpp` | `test/test_graphs_test.cpp` (or keep as-is as a framework file) | |
474+
| `test/lvalue_pmap.cpp` | `test/lvalue_pmap_test.cpp` | runtime entry in the Jamfile, not compile-only |
475+
| `test/max_flow_algorithms_bundled_properties_and_named_params.cpp` | `test/max_flow_algorithms_bundled_properties_and_named_params_test.cpp` | |
476+
| `test/mean_geodesic.cpp` | `test/mean_geodesic_test.cpp` | |
477+
| `test/metric_tsp_approx.cpp` | `test/metric_tsp_approx_test.cpp` | |
478+
| `test/min_degree_empty.cpp` | `test/min_degree_empty_test.cpp` | |
479+
| `test/rcsp_custom_vertex_id.cpp` | `test/rcsp_custom_vertex_id_test.cpp` | |
480+
| `test/rcsp_single_solution.cpp` | `test/rcsp_single_solution_test.cpp` | |
481+
| `test/read_propmap.cpp` | `test/read_propmap_test.cpp` | |
482+
| `test/sequential_vertex_coloring.cpp` | `test/sequential_vertex_coloring_test.cpp` | |
483+
| `test/serialize.cpp` | `test/serialize_test.cpp` | |
484+
| `test/subgraph.cpp` | `test/subgraph_test.cpp` | |
485+
| `test/subgraph_add.cpp` | `test/subgraph_add_test.cpp` | |
486+
| `test/subgraph_bundled.cpp` | `test/subgraph_bundled_test.cpp` | |
487+
| `test/subgraph_props.cpp` | `test/subgraph_props_test.cpp` | |
488+
| `test/tiernan_all_cycles.cpp` | `test/tiernan_all_cycles_test.cpp` | |
489+
| `test/undirected_dfs.cpp` | `test/undirected_dfs_test.cpp` | |
490+
491+
Explicit exceptions and deferrals:
492+
493+
- `test/test_graphs.cpp` is a framework-driver exception for now. Keep it as-is
494+
unless the suite is reworked enough to justify a clearer driver name.
495+
- Defer `test/transitive_closure_test2.cpp`,
496+
`test/vf2_sub_graph_iso_test_2.cpp`, and
497+
`test/weighted_matching_test2.cpp` to Step C-3, which decides whether each
498+
file is merged away or renamed to a descriptive `_test.cpp` name.
499+
- Do not count compile-only or currently unlisted files toward this runtime
500+
inventory. In the current tree that includes `copy.cpp`, `swap.cpp`,
501+
`property_iter.cpp`, `dimacs.cpp`, `filtered_graph_properties_dijkstra.cpp`,
502+
`undirected_dfs_visitor.cpp`, and `bidir_vec_remove_edge.cpp`.
488503

489504
Use `git mv` for each rename. Update all corresponding Jamfile entries.
490505

491-
Before marking this step complete, run a final audit and rename any remaining
492-
runtime test files that still do not follow the convention. Recommended checks:
506+
Before marking this step complete, rerun a programmatic audit and confirm that
507+
the remaining non-`_test.cpp` runtime files are only the explicit exception and
508+
the Step C-3 deferrals:
493509

494510
```sh
495-
rg --files test -g '*.cpp'
496-
rg -n '\[ (run|compile|compile-fail) ' test/Jamfile.v2
511+
python3 - <<'PY'
512+
from pathlib import Path
513+
import re
514+
515+
jam = Path('test/Jamfile.v2').read_text().splitlines()
516+
runtime = sorted({
517+
m.group(1)
518+
for line in jam
519+
for m in [re.match(r'\s*\[\s*run\s+([^\s\]]+\.cpp)\b', line)]
520+
if m
521+
})
522+
523+
allowed = {
524+
'test_graphs.cpp',
525+
'transitive_closure_test2.cpp',
526+
'vf2_sub_graph_iso_test_2.cpp',
527+
'weighted_matching_test2.cpp',
528+
}
529+
530+
remaining = [
531+
path for path in runtime
532+
if not path.endswith('_test.cpp') and not path.endswith('_cc.cpp')
533+
and path not in allowed
534+
]
535+
536+
for path in remaining:
537+
print(path)
538+
539+
raise SystemExit(1 if remaining else 0)
540+
PY
497541
```
498542

499543
After renaming, verify no other file in the repo `#include`s a renamed
@@ -573,7 +617,9 @@ The following entries are commented out in the Jamfile:
573617

574618
## Phase D — CMake parity and CI tiers
575619

576-
Depends on Phase B. Steps D-1 and D-2 can be done in a single PR.
620+
D-1 depends on Phase C (the CMakeLists uses post-C-1 filenames); D-2 depends
621+
only on D-1. Steps D-1 and D-2 can be done in a single PR once Phase C is
622+
complete.
577623

578624
---
579625

@@ -603,6 +649,14 @@ bgl_test(csr_graph_test)
603649
bgl_test(grid_graph_test)
604650
# ... (full list, one line per test)
605651
652+
# ---- Multi-variant tests (graph.cpp and property_iter.cpp) ----
653+
# bgl_test() cannot express these — write them out explicitly:
654+
add_executable(graph_1 graph_test.cpp)
655+
target_compile_definitions(graph_1 PRIVATE TEST=1)
656+
add_test(NAME graph_1 COMMAND graph_1)
657+
# repeat for graph_2 .. graph_9 and property_iter_1 .. property_iter_9
658+
# (property_iter is compile-only: use add_library(... OBJECT ...) with no add_test)
659+
606660
# ---- Concept checks (compile-only) ----
607661
add_library(adj_list_cc OBJECT concepts/adj_list_cc.cpp)
608662
# ... (one OBJECT target per *_cc.cpp)
@@ -764,7 +818,7 @@ complete.
764818
| C-2 | Migrate bare `assert` to Boost.Test | C-1 | not started | Only in files touched during C |
765819
| C-3 | Resolve `*2` suffix variants | C-1, E-2 | not started | Preserve distinct behavior before deleting any variant |
766820
| C-4 | Resolve commented-out tests | Phase B | not started | Do not delete files in this step |
767-
| D-1 | Create `test/CMakeLists.txt` | Phase B | not started | |
821+
| D-1 | Create `test/CMakeLists.txt` | Phase C | not started | CMakeLists uses post-C-1 filenames |
768822
| D-2 | Tag tests by cost | D-1 | not started | |
769823
| E-1 | Document `TEST=N` intent | none; must precede B-3 | not started | |
770824
| E-2 | Add coverage comments to `*2` files | none; must precede C-3 | not started | |

0 commit comments

Comments
 (0)