Bug(tests): Test#012 failure
- Dominant language
- JavaScript
- Stars
- 231
- Forks
- 242
- Avg merge
- 2h 51m
- Merged PRs (30d)
- 7
Description
# Bug Report
## Problem
```
1) pkgJson platform end-to-end with --save Test#012 : platform with
local path is added correctly with --save
- Unhandled promise rejection: TypeError: message.split is not a
function
at
- Error: Timeout - Async function did not complete within 150000ms
(set by jasmine.DEFAULT_TIMEOUT_INTERVAL)
at
at listOnTimeout (node:internal/timers:559:17)
at processTimers (node:internal/timers:502:7)
```
## Information
This is a two-part issue.
`message.split` is an error inside of Jasmine because Jasmine expects either a `string` or a `Error` type object to be passed as a rejection message. But some of our errors are a `CordovaError` object. Jasmine doesn't know how to use `CordovaError` and crashes when trying to `split` it and results with a test failure by timeout (rather than reporting the actual error).
This however, is not the focus of this ticket. The actual underlying error is:
```
- Error: CordovaError: Error: npm: Command failed with exit code 127 Error output:
npm ERR! code 127
npm ERR! path /tmp/cordova-lib-pkgJson-fAGV7s/android
npm ERR! command failed
npm ERR! command sh -c -- cordova-js build > templates/project/assets/www/cordova.js
npm ERR! sh: 1: cordova-js: not found
npm ERR! A complete log of this run can be found in:
npm ERR! /home/norman/.npm/_logs/2023-01-01T20_04_09_361Z-debug-0.log
npm ERR! code 127
npm ERR! path /tmp/cordova-lib-pkgJson-fAGV7s/android
npm ERR! command failed
npm ERR! command sh -c -- cordova-js build > templates/project/assets/www/cordova.js
npm ERR! sh: 1: cordova-js: not found
npm ERR! A complete log of this run can be found in:
npm ERR! /home/norman/.npm/_logs/2023-01-01T20_04_09_361Z-debug-0.log
at /development/cordova/coho/cordova-lib/integration-tests/pkgJson.spec.js:420:27
at processTicksAndRejections (node:internal/process/task_queues:96:5)
```
The test has a folder structure built that has 2 folders, `android/`, a `cordova-android` installation and `project/`, a cordova project for the purpose of the test.
There are two probems with `android/` cordova-android installation. `cordova-android` has a `prepare` lifecycle NPM script, primarily intended to trigger just before NPM "packs" the package. However NPM will trigger `prepare` in some situations, including when installing from git or when doing local installs.
#### Problem 1
Because this is a local directory, NPM is symlinking `android/` as `cordova-android` install, without installing it's dependencies, therefore `cordova-js` doesn't exists. If you enter `android/` and run NPM install it will correct this issue, but `cordova-js` will error leading into Problem 2
#### Problem 2
Even if `cordova-js` was installed, the `android/` structure is missing `cordova-js-src` directory and therefore cannot build the `cordova.js` file. The directory is missing because `android/` gets populated via an npm install / copy from `node_modules`. `cordova-js-src` is in the GIT repository, but is `.npmignore`'ed, because it's not required since the `cordova.js` file will be built before the tarball is packed. This is also evident because the `cordova.js` file does already exists.
#### The Root of the Problem
The described problem are a side effect of the root problem, we aren't testing the right **thing**. Test#012 is testing a situation that looks something like:
User retrievies an NPM packed tarball file (e.g. cordova-android.tgz).
User unpacks the tarball file.
User tries to `cordova platform add cordova-android/tarballDirectory`
This is a weird scenario to test and is likely unintentional. If the user has a tarball file, they should simply add the tarball file (e.g: `cordova platform add cordova-android.tgz`). They shouldn't manually extract the tarball and add the extracted directory. That doesn't make sense.
Installing a platform from a directory is typically done if someone has a development version of a platform, most likely from SVC repository like Git. Therefore the `android/` test artefact should reflect that kind of situation instead.
### Command or Code
If the test cleanup code is commented out so that the temporary files remain in tact after attempting to run them, you can manually trigger the error by attempting to add the `android/` directory as a platform, e.g: `cordova platform add ../android` (from within the `project/` folder)
### Environment, Platform, Device
Test Environment
### Version information
cordova-lib master / v11.1.0 (prerelease)
## Checklist
- [ ] I searched for existing GitHub issues
- [ ] I updated all Cordova tooling to most recent version
- [ ] I included all the necessary information above
Contributor guide
Research direction
Start with integration-tests/pkgJson.spec.js around Test#012 and the fixture's android/ and project/ directories; run the platform-add test and inspect the failure near line 420. Trace how the local cordova-android fixture is installed and prepared. Done means the test represents the intended directory-install scenario and reports a useful result without Jasmine timing out.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- mobile-dev, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100