You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by "kumo01GitHub (via GitHub)" <gi...@apache.org> on 2025/07/18 15:51:34 UTC
[PR] ci: drop file-url [cordova-fetch]
kumo01GitHub opened a new pull request, #126:
URL: https://github.com/apache/cordova-fetch/pull/126
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
n/a
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
`file-url` has switched to pure ESM in newer version and cannot be updated in dependencies. And `file-url` is used in test script only to convert path to file URL without file encoding. It looks excessive to use third-party node modules.
### Description
<!-- Describe your changes in detail -->
Drop `file-url` add `file://` to path in test script. At first I tried to use node default library `url.pathToFileURL()` but it causes error in windows because `url.pathToFileURL()` also converts unnecessary URL encoding.
### Testing
<!-- Please describe in detail how you tested your changes. -->
Ran `npm t`
### Checklist
- [x] I've run the tests to see all new and existing tests pass
- [x] I added automated test coverage as appropriate for this change
- [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [ ] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] ci: drop file-url [cordova-fetch]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #126:
URL: https://github.com/apache/cordova-fetch/pull/126#issuecomment-3170156759
## [Codecov](https://app.codecov.io/gh/apache/cordova-fetch/pull/126?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 92.18%. Comparing base ([`5acdbfd`](https://app.codecov.io/gh/apache/cordova-fetch/commit/5acdbfd9f90b2a07c9e59e7093ede163ea7e7f66?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`746dbd0`](https://app.codecov.io/gh/apache/cordova-fetch/commit/746dbd0af239b90cc6a1d9741325662056626387?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #126 +/- ##
=======================================
Coverage 92.18% 92.18%
=======================================
Files 1 1
Lines 64 64
=======================================
Hits 59 59
Misses 5 5
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cordova-fetch/pull/126?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] ci: drop file-url [cordova-fetch]
Posted by "erisu (via GitHub)" <gi...@apache.org>.
erisu commented on PR #126:
URL: https://github.com/apache/cordova-fetch/pull/126#issuecomment-3170404812
> At first I tried to use node default library url.pathToFileURL() but it causes error in windows because url.pathToFileURL() also converts unnecessary URL encoding.
Can you explain the issues you saw?
I tried testing with `pathToFileURL` and it was working. I ran my tests locally (macOS) and in GH Actions to confirm on Windows & Linux.
I think its OK to use `pathToFileURL` since we are only updating test code.
I made the following changes based on your current work:
Add to top below requiring `fs`:
```js
const { pathToFileURL } = require('node:url');
```
In test `should return package path if git repo name differs from plugin id` replace line:
```js
const TARGET = `git+file://${path.resolve(__dirname, 'support/repo-name-neq-plugin-id.git')}`;
```
with:
```js
const TARGET = `git+${pathToFileURL(path.resolve(__dirname, 'support/repo-name-neq-plugin-id.git'))}`;
```
In the `beforeEach` block of `fetching with node_modules in ancestor dirs`, replace line:
```js
fetchTarget = `file://${path.resolve('support/dummy-local-plugin')}`;
```
with:
```js
fetchTarget = pathToFileURL(path.resolve('support/dummy-local-plugin')).href;
```
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] ci: drop file-url [cordova-fetch]
Posted by "kumo01GitHub (via GitHub)" <gi...@apache.org>.
kumo01GitHub commented on PR #126:
URL: https://github.com/apache/cordova-fetch/pull/126#issuecomment-3172370947
[This](https://github.com/kumo01GitHub/cordova-fetch/tree/ci/drop-fileurl-win) is the branch I tried. I ran `Node CI` GH Actions and below failure occurred on Windows.
```
Failures:
1) fetching with node_modules in ancestor dirs should still install to given destination
Message:
Unhandled promise rejection: TypeError: message.split is not a function
Stack:
at <Jasmine>
Message:
Error: Timeout - Async function did not complete within 50000ms (custom timeout)
Stack:
at <Jasmine>
at listOnTimeout (node:internal/timers:608:17)
at process.processTimers (node:internal/timers:543:7)
```
I confirmed that `~` of `fetchTarget` URL is converted. And when I avoided to convert URL encoding, the failure was fixed.
```
fetchTarget: file:///C:/Users/RUNNER%7E1/AppData/Local/Temp/cordova-fetch-tests-AFo5vB/support/dummy-local-plugin
```
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] ci: drop file-url [cordova-fetch]
Posted by "kumo01GitHub (via GitHub)" <gi...@apache.org>.
kumo01GitHub commented on PR #126:
URL: https://github.com/apache/cordova-fetch/pull/126#issuecomment-3172385514
When I ran the test with shrinking timeout and intentionally cause timeout error on my local (MacOS), the error message was below and `TypeError` message didn't appear.
```
Failures:
1) fetching with node_modules in ancestor dirs should still install to given destination
Message:
Error: Timeout - Async function did not complete within 10ms (custom timeout)
Stack:
at <Jasmine>
at listOnTimeout (node:internal/timers:588:17)
at process.processTimers (node:internal/timers:523:7)
```
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] ci: drop file-url [cordova-fetch]
Posted by "kumo01GitHub (via GitHub)" <gi...@apache.org>.
kumo01GitHub commented on PR #126:
URL: https://github.com/apache/cordova-fetch/pull/126#issuecomment-3172380575
At first, I thought the failure is just timeout as you say and tried re-run some times. But finally I couldn't understand below message and expanding timeout had no effect. That's why I suspect there were other reason.
```
Unhandled promise rejection: TypeError: message.split is not a function
```
> If it is not an issue, can I try to push the changes I mentioned above to this PR?
Sure.
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] ci: drop file-url [cordova-fetch]
Posted by "erisu (via GitHub)" <gi...@apache.org>.
erisu commented on PR #126:
URL: https://github.com/apache/cordova-fetch/pull/126#issuecomment-3172373939
The error appears to be a timeout...
> Error: Timeout - Async function did not complete within 50000ms (custom timeout)
Could you try and re-run the failed tests to see if it passes? Might have to run it a couple of times...
I seen timeout failures before but were never able to figure out the exact cause. Usually just re-running the failed tests would pass.
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] test: drop file-url [cordova-fetch]
Posted by "erisu (via GitHub)" <gi...@apache.org>.
erisu merged PR #126:
URL: https://github.com/apache/cordova-fetch/pull/126
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
Re: [PR] ci: drop file-url [cordova-fetch]
Posted by "kumo01GitHub (via GitHub)" <gi...@apache.org>.
kumo01GitHub commented on PR #126:
URL: https://github.com/apache/cordova-fetch/pull/126#issuecomment-3089922247
After merged PR #125, I will open this PR.
--
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: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org