Skip to content

Resolve TODOs across 13 Python test files#6

Open
devin-ai-integration[bot] wants to merge 2 commits into
masterfrom
devin/1777288862-resolve-test-todos
Open

Resolve TODOs across 13 Python test files#6
devin-ai-integration[bot] wants to merge 2 commits into
masterfrom
devin/1777288862-resolve-test-todos

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Apr 27, 2026

Copy link
Copy Markdown

Reference Issues/PRs

Resolves multiple long-standing TODOs across the test suite. References issues man-group#1394, man-group#1537, man-group#1951.

What does this implement or fix?

Resolves 18 TODOs across 13 Python test files:

File Change
test_aggregation.py Add first and last aggregators to parametrize (now un-feature flagged)
test_string.py Remove Python 2 dead code branch and unused platform import
test_empty_column_type.py Extract head/tail processing pipeline tests into separate TestAppendNonesHeadTailPipeline class (5 TODOs)
storage_test.py Replace local create_df with import from arcticdb.util.test; remove resolved storage TODO
test_persistent_storage.py Add @pytest.mark.skipif for PERSISTENT_STORAGE_TESTS_ENABLED
test_engine.py Add row_range read verification to test_partial_write_non_contiguous
conftest.py Extend old_venv version params with 5.4.1, 5.9.3, 6.1.1
test_snapshot.py Remove stale lazy-fixture TODO (request.getfixturevalue is the standard pattern)
test_ternary.py Add float64 dtype assertion for projected column (modify_schema now merged)
test_arrow_sparse.py Add string/datetime column aggregation tests and offset resampling param
test_nonreg_specific.py Clean up update logic, remove stale dataframe_arctic_update TODO (man-group#1951 never merged)
test_append.py Remove stale commented-out AN-722 test case
test_s3.py (storage_fixtures) Fix rate limit cleanup with try/finally for test isolation

TODOs intentionally left in place (still valid / cannot resolve in Python tests):

  • mark.py: MACOS_CONDA_BUILD flag still in active use; SSL issue Study and add tests for SSL verification for S3 on Windows and MacOS man-group/ArcticDB#1394 still open
  • storage_test.py (lines 102, 121): Functions await inclusion in released package
  • test_arrow_read.py (5): Arrow write support still opt-in
  • test_compact_data.py (3): macOS timestamp resolution workaround
  • test_fork.py (2): Fork/multiprocessing infrastructure improvements
  • test_s3.py (integration): AWS InitAPI limitation
  • test_basic_version_store.py, test_library_tool.py, test_large_df.py, test_normalization.py, test_column_stats_creation.py, date.py: C++ layer / infrastructure issues

Any other comments?

No API changes. All changes are test-only.

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

Link to Devin session: https://app.devin.ai/sessions/2e16302813274601af9de196433618f2
Requested by: @chrischappell-cog


Open in Devin Review

devin-ai-integration Bot and others added 2 commits April 27, 2026 11:30
- test_aggregation.py: Add 'first' and 'last' aggregators to parametrize (now un-feature flagged)
- test_string.py: Remove Python 2 dead code branch and unused platform import
- test_empty_column_type.py: Extract head/tail processing pipeline tests into separate TestAppendNonesHeadTailPipeline class
- storage_test.py: Replace local create_df with import from arcticdb.util.test; remove resolved storage TODO
- test_persistent_storage.py: Add skipif check for PERSISTENT_STORAGE_TESTS_ENABLED
- test_engine.py: Add row_range read verification to test_partial_write_non_contiguous
- conftest.py: Extend old_venv version params with 5.4.1, 5.9.3, 6.1.1
- test_snapshot.py: Remove lazy-fixture TODO (request.getfixturevalue is the standard pattern)
- test_ternary.py: Add float64 dtype assertion for projected column (modify_schema now merged)
- test_arrow_sparse.py: Add string/datetime column tests and offset resampling param
- test_nonreg_specific.py: Clean up update logic, remove stale TODO
- test_append.py: Remove stale commented-out AN-722 test case
- test_s3.py (storage_fixtures): Fix rate limit cleanup with try/finally for test isolation

Co-Authored-By: Chris Chappell <chris.chappell@cognition.ai>
Co-Authored-By: Chris Chappell <chris.chappell@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 7 additional findings.

Open in Devin Review

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.

1 participant