Skip to content

[Stack 1/25] Test cleanup: consolidate Clojure comparison tests#2417

Merged
jucor merged 3 commits intoedgefrom
jc/pca_test_cleanup
Mar 19, 2026
Merged

[Stack 1/25] Test cleanup: consolidate Clojure comparison tests#2417
jucor merged 3 commits intoedgefrom
jc/pca_test_cleanup

Conversation

@jucor
Copy link
Copy Markdown
Collaborator

@jucor jucor commented Mar 5, 2026

Summary

Next in stack: #2418 (Clojure comparison test consolidation and cleanup)

Based on edge. #2413#2416 have been merged.
Next in stack: #2418 (Clojure comparison consolidation)

Post-sklearn cleanup:

  • Consolidate Clojure comparison tests into test_legacy_clojure_regression.py, delete redundant test_golden_data.py
  • Change @skip markers to @xfail for tests with known differences

Test plan

  • 198 passed, 3 skipped, 4 xfailed
    🤖 Generated with Claude Code

…ion.py

- Delete test_golden_data.py: redundant with test_legacy_clojure_regression.py
- Change @Skip to @xfail for test_group_clustering and test_comment_priorities
  so they run and document expected failures rather than being silently skipped

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@jucor jucor force-pushed the jc/pca_test_cleanup branch from 359a2dd to ecd5b9b Compare March 10, 2026 12:29
@jucor jucor marked this pull request as draft March 10, 2026 13:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Cleans up legacy Clojure comparison testing by removing a redundant golden-data test and switching two placeholder “not implemented yet” checks from being skipped to being tracked as expected failures.

Changes:

  • Convert two legacy Clojure comparison tests from @pytest.mark.skip to @pytest.mark.xfail.
  • Delete test_golden_data.py (redundant golden-data comparison test).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
delphi/tests/test_legacy_clojure_regression.py Switches two legacy Clojure comparison tests to xfail so they execute and report expected failures.
delphi/tests/test_golden_data.py Removes a redundant xfailed golden-data comparison test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Unconstrained xfail swallows any exception (KeyError, AttributeError, etc.),
masking real regressions. Adding raises=AssertionError ensures only assertion
failures are treated as expected, and strict=True alerts when tests start passing.

Addresses GitHub Copilot review feedback on PR #2417.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jucor jucor marked this pull request as ready for review March 10, 2026 14:05
@jucor jucor changed the title Test cleanup: consolidate Clojure comparison tests [Stack 1/8] Test cleanup: consolidate Clojure comparison tests Mar 10, 2026
@jucor jucor changed the title [Stack 1/8] Test cleanup: consolidate Clojure comparison tests [Stack 1/9] Test cleanup: consolidate Clojure comparison tests Mar 11, 2026
@jucor jucor changed the title [Stack 1/9] Test cleanup: consolidate Clojure comparison tests [Stack 1/10] Test cleanup: consolidate Clojure comparison tests Mar 11, 2026
@jucor jucor changed the title [Stack 1/10] Test cleanup: consolidate Clojure comparison tests [Stack 1/11] Test cleanup: consolidate Clojure comparison tests Mar 11, 2026
@jucor jucor changed the title [Stack 1/11] Test cleanup: consolidate Clojure comparison tests [Stack 1/12] Test cleanup: consolidate Clojure comparison tests Mar 13, 2026
@jucor jucor changed the title [Stack 1/12] Test cleanup: consolidate Clojure comparison tests [Stack 1/13] Test cleanup: consolidate Clojure comparison tests Mar 13, 2026
@jucor jucor changed the title [Stack 1/13] Test cleanup: consolidate Clojure comparison tests [Stack 1/15] Test cleanup: consolidate Clojure comparison tests Mar 16, 2026
Stacked PRs target jc/* branches, not edge/stable, so the Python CI
was not running on them. Add jc/** to pull_request.branches so stack
PRs get test coverage. Push triggers remain edge/stable only.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jucor jucor changed the title [Stack 1/15] Test cleanup: consolidate Clojure comparison tests [Stack 1/16] Test cleanup: consolidate Clojure comparison tests Mar 16, 2026
@github-actions
Copy link
Copy Markdown

Delphi Coverage Report

File Stmts Miss Cover
init.py 3 0 100%
main.py 55 55 0%
benchmarks/bench_pca.py 76 76 0%
benchmarks/bench_repness.py 81 81 0%
benchmarks/bench_update_votes.py 38 38 0%
benchmarks/benchmark_utils.py 34 34 0%
components/init.py 2 0 100%
components/config.py 165 133 19%
components/server.py 116 72 38%
conversation/init.py 2 0 100%
conversation/conversation.py 1036 353 66%
conversation/manager.py 131 42 68%
database/init.py 1 0 100%
database/dynamodb.py 387 234 40%
database/postgres.py 306 205 33%
pca_kmeans_rep/init.py 5 0 100%
pca_kmeans_rep/clusters.py 234 8 97%
pca_kmeans_rep/corr.py 98 17 83%
pca_kmeans_rep/pca.py 50 15 70%
pca_kmeans_rep/repness.py 361 48 87%
pca_kmeans_rep/stats.py 107 22 79%
poller.py 224 188 16%
regression/init.py 4 0 100%
regression/comparer.py 887 406 54%
regression/datasets.py 95 21 78%
regression/recorder.py 36 27 25%
regression/utils.py 137 38 72%
run_math_pipeline.py 260 239 8%
system.py 85 55 35%
umap_narrative/500_generate_embedding_umap_cluster.py 210 109 48%
umap_narrative/501_calculate_comment_extremity.py 112 54 52%
umap_narrative/502_calculate_priorities.py 135 135 0%
umap_narrative/700_datamapplot_for_layer.py 502 502 0%
umap_narrative/701_static_datamapplot_for_layer.py 310 310 0%
umap_narrative/702_consensus_divisive_datamapplot.py 432 432 0%
umap_narrative/801_narrative_report_batch.py 787 787 0%
umap_narrative/802_process_batch_results.py 265 265 0%
umap_narrative/803_check_batch_status.py 175 175 0%
umap_narrative/llm_factory_constructor/init.py 2 2 0%
umap_narrative/llm_factory_constructor/model_provider.py 157 157 0%
umap_narrative/polismath_commentgraph/init.py 1 0 100%
umap_narrative/polismath_commentgraph/cli.py 270 270 0%
umap_narrative/polismath_commentgraph/core/init.py 3 3 0%
umap_narrative/polismath_commentgraph/core/clustering.py 110 110 0%
umap_narrative/polismath_commentgraph/core/embedding.py 104 104 0%
umap_narrative/polismath_commentgraph/lambda_handler.py 219 219 0%
umap_narrative/polismath_commentgraph/schemas/init.py 2 0 100%
umap_narrative/polismath_commentgraph/schemas/dynamo_models.py 160 9 94%
umap_narrative/polismath_commentgraph/tests/conftest.py 17 17 0%
umap_narrative/polismath_commentgraph/tests/test_clustering.py 74 74 0%
umap_narrative/polismath_commentgraph/tests/test_embedding.py 55 55 0%
umap_narrative/polismath_commentgraph/tests/test_storage.py 87 87 0%
umap_narrative/polismath_commentgraph/utils/init.py 3 0 100%
umap_narrative/polismath_commentgraph/utils/converter.py 283 237 16%
umap_narrative/polismath_commentgraph/utils/group_data.py 354 336 5%
umap_narrative/polismath_commentgraph/utils/storage.py 585 477 18%
umap_narrative/reset_conversation.py 159 50 69%
umap_narrative/run_pipeline.py 453 312 31%
utils/general.py 63 41 35%
Total 11105 7736 30%

@jucor jucor changed the title [Stack 1/16] Test cleanup: consolidate Clojure comparison tests [Stack 1/17] Test cleanup: consolidate Clojure comparison tests Mar 16, 2026
@jucor jucor changed the title [Stack 1/17] Test cleanup: consolidate Clojure comparison tests [Stack 1/24] Test cleanup: consolidate Clojure comparison tests Mar 17, 2026
@jucor jucor changed the title [Stack 1/24] Test cleanup: consolidate Clojure comparison tests [Stack 1/25] Test cleanup: consolidate Clojure comparison tests Mar 17, 2026
Copy link
Copy Markdown
Collaborator

@whilo whilo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Collaborator

@whilo whilo left a comment

Choose a reason for hiding this comment

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

Hehe, Claude decided to comment, nvm.

@jucor jucor changed the title [Stack 1/25] Test cleanup: consolidate Clojure comparison tests [Stack 1/24] Test cleanup: consolidate Clojure comparison tests Mar 19, 2026
jucor added a commit that referenced this pull request Mar 19, 2026
Unconstrained xfail swallows any exception (KeyError, AttributeError, etc.),
masking real regressions. Adding raises=AssertionError ensures only assertion
failures are treated as expected, and strict=True alerts when tests start passing.

Addresses GitHub Copilot review feedback on PR #2417.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jucor jucor changed the title [Stack 1/24] Test cleanup: consolidate Clojure comparison tests [Stack 1/25] Test cleanup: consolidate Clojure comparison tests Mar 19, 2026
@jucor jucor merged commit 748bc8a into edge Mar 19, 2026
4 checks passed
@jucor jucor deleted the jc/pca_test_cleanup branch March 19, 2026 14:59
@jucor jucor restored the jc/pca_test_cleanup branch March 19, 2026 15:21
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.

4 participants