Skip to content

[CLN]: remove CHROMA_RUST_BINDINGS_TEST_ONLY#4048

Closed
codetheweb wants to merge 1 commit intocln-update-py-fastapi-fixturesfrom
cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY
Closed

[CLN]: remove CHROMA_RUST_BINDINGS_TEST_ONLY#4048
codetheweb wants to merge 1 commit intocln-update-py-fastapi-fixturesfrom
cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY

Conversation

@codetheweb
Copy link
Contributor

@codetheweb codetheweb commented Mar 21, 2025 •
edited
Loading

Description of changes

Tests against the Rust bindings in Python are now run by default.

@github-actions
Copy link

github-actions bot commented Mar 21, 2025

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

@codetheweb Graphite App
Copy link
Contributor Author

codetheweb commented Mar 21, 2025 •
edited
Loading

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codetheweb codetheweb mentioned this pull request Mar 21, 2025
Closed
1 task
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from d5650d3 to 8363e9f Compare March 21, 2025 19:16
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from b201624 to b6601b7 Compare March 21, 2025 19:16
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 8363e9f to 023a439 Compare March 21, 2025 20:05
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from b6601b7 to f9f3d4d Compare March 21, 2025 20:05
HammadB
HammadB reviewed Mar 21, 2025
View reviewed changes
.github/workflows/_python-tests.yml
PROPERTY_TESTING_PRESET: ${{ inputs.property_testing_preset }}
CHROMA_RUST_BINDINGS_TEST_ONLY: "1"

test-rust-single-node-integration:
Copy link
Collaborator

@HammadB HammadB Mar 21, 2025

Choose a reason for hiding this comment

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

why are you removing this?

Copy link
Contributor Author

@codetheweb codetheweb Mar 21, 2025

Choose a reason for hiding this comment

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

we have a test fixture that starts a HTTP server using the CLI, so this is redundant

Copy link
Collaborator

@HammadB HammadB Mar 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, I think we should test the docker container then. That was valuable to do in CI for python.

Also is this new? We didn't have this before (Assuming you mean its created in - #4037)

Copy link
Contributor Author

@codetheweb codetheweb Mar 21, 2025

Choose a reason for hiding this comment

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

we can certainly add some smoke tests against the container, but running the full suite seems unnecessary
the Rust HTTP server fixture was added in #4037

Copy link
Collaborator

@HammadB HammadB Mar 21, 2025

Choose a reason for hiding this comment

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

Yes I just said that ty though.

Copy link
Collaborator

@HammadB HammadB Mar 21, 2025

Choose a reason for hiding this comment

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

Agree on smoke tests

Copy link
Contributor Author

@codetheweb codetheweb Mar 21, 2025

Choose a reason for hiding this comment

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

cool I'll cut a ticket for container smoke tests

@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 023a439 to 68ada1d Compare March 21, 2025 20:26
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from f9f3d4d to deacdf1 Compare March 21, 2025 20:26
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 68ada1d to dcf9fa0 Compare March 21, 2025 20:35
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from deacdf1 to 0b7bc07 Compare March 21, 2025 20:35
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from dcf9fa0 to 3689742 Compare March 21, 2025 20:58
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from 0b7bc07 to d61a0fc Compare March 21, 2025 20:58
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 3689742 to 26a865c Compare March 21, 2025 23:08
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from d61a0fc to 87b041f Compare March 21, 2025 23:08
@codetheweb codetheweb marked this pull request as ready for review March 21, 2025 23:16
@codetheweb codetheweb requested a review from HammadB March 21, 2025 23:16
@codetheweb codetheweb mentioned this pull request Mar 21, 2025
Closed
1 task
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 26a865c to 42ace1a Compare March 22, 2025 00:17
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from 87b041f to 3360f78 Compare March 22, 2025 00:17
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 42ace1a to f3e13e1 Compare March 22, 2025 01:09
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from 3360f78 to 3ef3c05 Compare March 22, 2025 01:10
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from f3e13e1 to 6f2f402 Compare March 24, 2025 16:57
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from 3ef3c05 to d5eb8c4 Compare March 24, 2025 16:57
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 6f2f402 to eb1a2ce Compare March 26, 2025 21:45
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from d5eb8c4 to 9c5846b Compare March 26, 2025 21:45
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from eb1a2ce to 6597e90 Compare March 26, 2025 22:01
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from 9c5846b to 8e9d730 Compare March 26, 2025 22:01
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 6597e90 to 780b86c Compare March 27, 2025 17:12
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from 8e9d730 to 8e41214 Compare March 27, 2025 17:12
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 780b86c to 33035f2 Compare March 27, 2025 17:57
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from 8e41214 to cb5f9ef Compare March 27, 2025 17:58
@jairad26 jairad26 changed the base branch from cln-update-py-fastapi-fixtures to graphite-base/4048 March 27, 2025 19:52
@jairad26 jairad26 force-pushed the graphite-base/4048 branch from 33035f2 to f7a36c7 Compare March 27, 2025 19:58
@jairad26 jairad26 force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from cb5f9ef to 6bc0f23 Compare March 27, 2025 19:58
@jairad26 jairad26 changed the base branch from graphite-base/4048 to cln-update-py-fastapi-fixtures March 27, 2025 19:58
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from f7a36c7 to 942e032 Compare March 27, 2025 23:22
@codetheweb codetheweb force-pushed the cln-update-py-fastapi-fixtures branch from 942e032 to 9dacf02 Compare March 27, 2025 23:23
@codetheweb codetheweb force-pushed the cln-remove-CHROMA_RUST_BINDINGS_TEST_ONLY branch from 6bc0f23 to 872e5fb Compare March 27, 2025 23:23
@HammadB HammadB closed this Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HammadB HammadB Awaiting requested review from HammadB

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@codetheweb @HammadB

Comments