chore(api-tests): honour --base-url / VNEXT_BASE_URL and refresh runsettings wording - #18
Conversation
…s and refresh runsettings wording - Every script under api-tests/ that hard-coded http://localhost:4201 now takes --base-url, defaulting to the VNEXT_BASE_URL environment variable and then to localhost:4201, so the behaviour/load tests can target a second domain or offset (e.g. partner on :4211) like the integration tests already can. - publish.py (script-race-lab, script-perf-lab) gained main(argv=None, base_url=None): race-load.py and perf-load.py load it in-process and would otherwise have fed their own argv into its parser; they now pass argv=[] and their --base-url. - TEST-SCENARIOS.md, Tests/FanOut, Tests/RoleMatrixLab and api-tests/fan-out-documents READMEs no longer tell the reader to "uncomment" VNEXT_BASE_URL — it has been committed as set for a while; they now say to verify it and to use the git-ignored test.runsettings.local for a different port, with the SDK's precedence order. Companion of burgan-tech/vnext#974 (integration-test contract + runtime-integration-test skill). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Reviewer's GuideThe PR makes all targeted API-test scripts honor VNEXT_BASE_URL or an explicit --base-url, including safe propagation through in-process publish calls, and updates integration-test documentation to reflect the committed runsettings value and local override workflow. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="api-tests/script-perf-lab/perf-load.py" line_range="340" />
<code_context>
if args.publish:
print("Publish:")
- if not publish():
+ if not publish(args.base_url):
return 1
</code_context>
<issue_to_address>
**issue (broader_impact):** `perf-load.py` still defaults its own `--base-url` to `http://localhost:4201`, so when only `VNEXT_BASE_URL` is set, `publish(args.base_url)` publishes to localhost instead of the configured orchestrator. The subsequent performance requests use the same hard-coded default and also ignore `VNEXT_BASE_URL`.
**Triggers:** When `perf-load.py` is run without `--base-url` and `VNEXT_BASE_URL` points to a non-default orchestrator.
**Suggested fix:** Give `perf-load.py` the same `DEFAULT_BASE_URL = os.environ.get(...)` default as the other scripts and use it for its argument parser.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the scripts now derive their API target from an environment variable or command-line argument, so a misconfigured value can publish definitions and run workflow tests against an unintended runtime. Reverting restores the previous localhost behavior, but any records or other test-side effects already created on the wrong runtime would need to be cleaned up separately.
Blocking findings: api-tests/script-perf-lab/perf-load.py:340
|
|
||
| if args.publish: | ||
| print("Publish:") | ||
| if not publish(): |
There was a problem hiding this comment.
issue (broader_impact): perf-load.py still defaults its own --base-url to http://localhost:4201, so when only VNEXT_BASE_URL is set, publish(args.base_url) publishes to localhost instead of the configured orchestrator. The subsequent performance requests use the same hard-coded default and also ignore VNEXT_BASE_URL.
Triggers: When perf-load.py is run without --base-url and VNEXT_BASE_URL points to a non-default orchestrator.
Suggested fix: Give perf-load.py the same DEFAULT_BASE_URL = os.environ.get(...) default as the other scripts and use it for its argument parser.
1 participant
Summary
api-tests/Python scripts that hard-codedhttp://localhost:4201now accept--base-url, defaulting toVNEXT_BASE_URLand thenlocalhost:4201, so behaviour/load tests can target another domain or offset the same way the xUnit suite already does.race-load.pyandperf-load.pyload their siblingpublish.pyand callmain(); with argparse added,main(argv=None, base_url=None)keeps that path working.VNEXT_BASE_URL" wording inTEST-SCENARIOS.mdand three READMEs — the value is committed as set; the personal override lever is the git-ignoredtest.runsettings.local.Changes
api-tests/chain-busy/*.py,data-integrity-lab/integrity-lab-test.py,l1-cache-lab/l1-cache-behaviour-test.py,script-race-lab/{race-load,publish}.py,script-perf-lab/{perf-load,publish}.py,secret-cache-lab/secret-cache-behaviour-test.py,subflow-orchestration/updatedata-concurrency-test.pyTEST-SCENARIOS.md,tests/Core.IntegrationTests/Tests/FanOut/README.md,tests/Core.IntegrationTests/Tests/RoleMatrixLab/README.md,api-tests/fan-out-documents/README.mdTest Plan
python3 -m py_compileand--helpon all nine scripts.VNEXT_BASE_URL=http://partner:4211/→ moduleBASEresolves tohttp://partner:4211/api/v1(two scripts checked).publish.main(argv=[], base_url=...)with a foreignsys.argvraises a connection error, not an argparse error.python3 api-tests/chain-busy/chain-busy-behaviour-test.py --listagainst a running runtime (behaviour unchanged on the default URL).Notes
Companion of burgan-tech/vnext#974, which commits the integration-test contract (
docs/testing/integration-testing.md) and theruntime-integration-testskill that points at these scripts.🤖 Generated with Claude Code
Summary by Sourcery
Allow API tests to run against configurable runtime endpoints while refreshing integration-test configuration guidance.
New Features:
--base-urlorVNEXT_BASE_URL.Bug Fixes:
Enhancements:
VNEXT_BASE_URLsetting should remain enabled and that local overrides belong in the ignored runsettings file.Documentation:
Tests: