weijinglin commented on PR #315: URL: https://github.com/apache/hugegraph-ai/pull/315#issuecomment-4097283336
> Thanks for the contribution. I reviewed this PR and found two issues that should be addressed before merge: > > 1. [P1] New integration test is not executed in CI > > * File: .github/workflows/hugegraph-llm.yml (Run integration tests step) > * test_flows_integration.py is added in this PR, but the workflow still runs only three explicit integration files. So the new tests are not part of PR checks and provide no regression protection. > * Suggestion: include the new file in the pytest command, or switch to directory-level collection with markers. > > 2. [P2] Missing external-service skip guard in new integration tests > > * File: hugegraph-llm/src/tests/integration/test_flows_integration.py > * The tests call real flows directly (BUILD_VECTOR_INDEX / GRAPH_EXTRACT / RAG_* / TEXT2GREMLIN) without should_skip_external()-style gating or mocks. Once enabled in CI/local integration runs, they can become flaky and environment-dependent. > * Suggestion: add explicit skip conditions for external dependencies, or stabilize with mocks/stubs for non-deterministic external calls. > > Please address these and I can re-review quickly. Thank u for your suggestions. For P1, I plan to add a end-to-end test for all the test cases in the app demo, which need api-key to launch the basic test. So I don't add the test scripts to the CI. I wanna add a contributed.md for this project and call for all the contributor to check the basic functionality of this project. What about your opinion? For P2, I don't simulate the external api because of the complexity of the workflow (caused by the nondeterminism of llm), so this test scripts is not added to the CI procedure. I'm sorry I didn't reply to you in a timely manner. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
