cap-js-community / odata-v2-adapter

OData V2 adapter for CDS. Exposes a full-fledged OData V2 service, converting OData V2 requests to CDS OData V4 service calls and responses back.
https://www.npmjs.com/package/@cap-js-community/odata-v2-adapter
Apache License 2.0
23 stars 8 forks source link

fix: use cds.root as location for package.json #20

Closed johannes-vogel closed 10 months ago

johannes-vogel commented 10 months ago

process.cwd() is dependent on the location of the process, while cds.root always points to the project root. Most often and by default cds.root points to process.cwd() but in the context of cds.test the depending on the directory of the process, the wrong package.json might be read.

oklemenz2 commented 10 months ago

Thanks for the fix. What I don't understand, how this causes a regression in your tests. Because this just enables the plugin, and that option was not existing before. So I cannot imagine what did break...

oklemenz2 commented 10 months ago

PRs based on a fork from different repo/org will not pass integration-tests, as secrets are not injected: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

oklemenz2 commented 10 months ago

Fixed with https://github.com/cap-js-community/odata-v2-adapter/commit/67dc513c2e762267d26321104e361de58af4d4cf

oklemenz2 commented 10 months ago

@johannes-vogel next time just open PR based on cap-js-community, then it can be directly merged...

johannes-vogel commented 10 months ago

@johannes-vogel next time just open PR based on cap-js-community, then it can be directly merged...

will do so next time. Richard only recently added me to the community org.

The issue popped up in our (maybe not ideally written) tests and cwd was != cds.root and cwd did not contain a package.json which resulted in a crash.

oklemenz2 commented 10 months ago

OK, I See, I will make that more robust as well.