test: configure via .cdsrc.json profiles instead of process.env.cds_* (#486) - #493
Conversation
SummaryThe following content is AI-generated and provides a summary of the pull request: Replace
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR is a clean refactoring that correctly replaces load-order-fragile process.env.cds_* overrides with declarative .cdsrc.json profiles. The only issue found is a minor inaccuracy in the logging.test.js comment that conflates the moved base value (["tenant_id"]) with the profile-level override (["foo"]), which could mislead future readers. One comment was posted to clarify this.
PR Bot Information
Version: 1.29.33
- Correlation ID:
e85349f0-9ccc-11f1-8c5f-fd242520031d - LLM:
anthropic--claude-4.6-sonnet - File Content Strategy: Full file content
- Event Trigger:
pull_request.opened
| // - log.cls_custom_fields: ['foo'] (the base default from package.json's cds.log moved to | ||
| // .cdsrc.json so this profile can win — see #486; package.json config loads after .cdsrc.json | ||
| // and would otherwise override the profile) |
There was a problem hiding this comment.
Inaccuracy: The comment says log.cls_custom_fields: ['foo'] is "the base default from package.json's cds.log moved to .cdsrc.json". What actually moved from package.json to the .cdsrc.json base is ["tenant_id"]; ["foo"] is the [logging] profile override — it was never in package.json. The comment conflates the two and will mislead future readers about what the base value is and why the profile wins.
| // - log.cls_custom_fields: ['foo'] (the base default from package.json's cds.log moved to | |
| // .cdsrc.json so this profile can win — see #486; package.json config loads after .cdsrc.json | |
| // and would otherwise override the profile) | |
| // - log.cls_custom_fields: ['foo'] (profile override; the base default ['tenant_id'] was moved | |
| // from package.json into the .cdsrc.json base so this profile can win — see #486; | |
| // package.json config loads after .cdsrc.json and would otherwise override the profile) |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
1 participant
What
Replaces the load-order-fragile
process.env.cds_requires_*/process.env.cds_*string-JSON test config with proper cds config:.cdsrc.jsonprofiles (composed with--profile) andcds.test()args. Test-only + test-app-config change; nolib/change.Why
Setting config via
process.env.cds_*string-JSON at module top is load-order-sensitive: it must run before any@sap/cdsrequire or it is silently ignored (the root of the removeddelete cds.envhack). cds already supports the same config declaratively via.cdsrc.jsonprofiles.Root cause of the old
// REVISIT: ... package.json winscomments (why some config had to be done via env): cds loads config sources sequentially,package.jsonafter.cdsrc.json, last-writer-wins. So a.cdsrc.jsonprofile could never override a key thatpackage.jsonset at its base. Fix: move those base defaults (log.cls_custom_fields,messaging.kind/file) fromtest/bookshop/package.jsonintotest/bookshop/.cdsrc.jsonbase, so the.cdsrc.jsonprofiles (same source) win as intended.Sites moved to profiles (all 8 — zero
process.env.cds_*remain)logging.test.js(cls_custom_fields, tracing.exporter:false)cds_log,cds_requires_telemetry_tracing[logging]profile (+ base moved out of package.json)tracing.test.js(sampler ignoreIncomingPaths)cds_requires_telemetry_tracing_sampler[sampler-ignore-authors]profile,--profile 'tracing-in-memory, sampler-ignore-authors'tracing-remote-native.test.js,tracing-attributes.test.js(native_fetch)cds_remote_native__fetch[native-fetch]profile (remote.native_fetch: true),--profile 'tracing-in-memory, native-fetch'passport.test.js(scheduling off)cds_requires_scheduling[no-scheduling]profile,cds.test(dir, '--profile', 'no-scheduling')tracing-messaging-{inboxed,persistent-outbox,without-outbox}.test.js(messaging kind/file/flags)cds_requires_messaging[inboxed]/[persistent-outbox]/[without-outbox]profiles now fully carry it (messaging base moved out of package.json).cdsrc.json changes
requires.messaging(local-messaging / msg-box) andlog.cls_custom_fields(moved from package.json).[logging]: addedrequires.telemetry.tracing.exporter: false.[sampler-ignore-authors],[native-fetch],[no-scheduling].Verification
grep -rn "process.env.cds_" test/→ none remain.grep -c int.repositories.cloud.sap package-lock.json= 0.HANA CI caveat
Profile changes are DB-agnostic, but these run on HANA and could only be validated for config equivalence locally (config resolves identically to the old env), not executed:
logging(runs on both), and the HANA-onlytracing-messaging-inboxed+tracing-messaging-persistent-outbox. HANA CI must validate them.No lib change. No behavior change.
Closes #486