'npm link' is weird and we do not need to use it. `npm install /path/to/some/module` will create a symbolic link, but it _won't_ create a global link. it otherwise behaves similarly to `npm install`.
- also updated to use `@types/teen_process`
- fixed some weird types in `util.getLockFileGuard()`
Most of these changes are the addition of types. There are a few places in which the typechecker found weirdness, and I have addressed those.
- In `logging.js`, `parseInt()` was being called on props of `process.env`; this is not allowed, because `parseInt()` only accepts a string, and props may be `undefined`.
- In `log-internal.js`, I had to remove the `raiseError()` function because TypeScript is unable to understand that a function, when called, will always throw. The fix was just to replace with `throw new Error...`
- Introduced a var in `mjpeg.js` to avoid repeated casting
- Added some type guards to `net.js`. as function parameters, union types kind of suck. This is definitely one place TS would have helped, since it allows function overloads. Also dropped use of deprecated `url` module in lieu of W3C `URL`.
- Removed some old-node-version cruft from `util.js`.
Non-code changes:
- Needed to use an override for `type-fest`, as it's included as a transitive dependency, but otherwise is only present in the devdeps of the monorepo root `package.json`. This means, due to version conflicts, a `support/node_modules/type-fest` would be created with an old, incompatible version, which would be preferred over the monorepo one due to Node's resolution algorithm. This needs npm v8.3+ to work properly, but is only a problem for contributors (not consumers).
- Pinned type of `opencvv-bindings`.
- No longer deleting `build` dir before transpilation since it may contain generated declarations!
This PR builds and tests `@appium/support` without Gulp. Much like the changes to `appium`:
- Test files reorganized and renamed
- Tests run via `mocha` proper using `@babel/register` (no build needed)
- Build run via `@babel/cli`
* chore: ban direct usage of sinon
Calling `sinon.stub()`, `sinon.mock()` etc., directly is error-prone. Care must be taken to explicitly restore any stub created.
Using a sandbox (via `sinon.createSandbox()`), you don't need to remember to restore everything you fiddled with.
To that end, I've converted all direct `sinon` calls into sandbox calls and **BANNED** direct usage of `sinon` via ESLint.
# Conflicts:
# .eslintrc
* test(base-driver): replace restricted sinon.spy() usage in server-specs.js
* test(support): formatting
This change removes `gulp` and replaces it with plain ol' `babel` and `mocha`.
## `appium`
- No gulp.
- `test/` has been re-organized. All test files now end in the extension `.spec.js`, which is a recognized convention and understood by editors/IDEs. The tests are further split into `unit` and `e2e` subdirs. This makes working with both `babel` and `mocha` easier.
- The tests are _not_ run against the transpiled code; code is now transpiled on-the-fly via `@babel/register`. This only affects `appium`.
- `commands-yml/validator.js` moved to `scripts/parse-yml-commands.js`. It has been rewritten as a CJS module. `commands-yml/` should not contain `.js` files.
- `test/setup.js` is a new test harness specific to Appium, which loads & configures `@babel/register`. Eventually, the contents of this file can be moved into the root monorepo's `test/setup.js`, but the rest of the packages do not need `@babel/register`, so we avoid the overhead.
## `@appium/gulp-plugins`
- Unfortunately, due to the new `npm test` strategy (see below), we cannot use the `nyan` reporter. Or rather, we _can_, but will not see the animation. So the reporter is now set to default to `spec`.
- Modified tests so they didn't overwrite the actual `build` dir with transpiled fixtures, which is what was happening. This was causing failures depending on the transpile/test order.
## All Packages
- All packages now have their own `build`, `test`, `test:e2e` and `dev` (just build + watch mode) npm scripts. This makes running a complete build easier, since each package can now provide its own appropriate command. This is _mostly_ just `gulp once` (where `gulp` was used before), except for `appium`, where `@babel/cli` is used directly.
- This is what happens when `npm test` is run from the monorepo (in order):
1. All packages are built (in parallel) by running `npm run build` in each folder (via `lerna`). See `pretest`
2. All code is linted directly via `eslint`.
3. All _unit tests_ are run w/ `npm test` in each folder, having assumed transpilation already occurred (if necessary).
- `npm run e2e-test` runs the `npm run test:e2e` in each folder. No transpilation occurs! Careful.
- All scripts are expected to live in `packages/*/scripts`, and this directory enforces CJS via ESLint. `postinstall.js` moved, as well as `check-npm-pack-files.js`. `generate-schema-declarations.mjs` moved from the root `scripts/` dir into `packages/appium/scripts/`; it's now CJS instead of ESM.
- I had profiled running various things with `lerna run --parallel`, but it turns out this is not faster. I'm not sure why, but I think it may have something to do with streaming STDOUT/STDERR, or potentially the overhead of running child processes outweighs the time it takes to actually run tests and builds.
- The behavior of `lerna run` now mimicks `lerna exec`, which was "run stuff in serial, streaming the output". The default behavior of `lerna run` is "run stuff in parallel, buffering the output". Buffered output sucks, and streaming the output from all tasks at once creates chaos. To override this behavior, you'll see `--parallel --concurrency=8 --prefix` for tasks that can truly be parallelized, like transpilation. The number 8 is the "maximum", and CI usually won't have more than 2 cores available anyway.
- When run in CI, Mocha runs with `--forbid-only`, which will fail if there's an `.only()` somewhere in the tests. Further, it forces color output.
## Etc.
- Updated Wallaby config, `.gitignore`, `.eslintignore`.
- Sorted `package.json` files
- Removed `watch` and `coverage` scripts which were broken anyway.
chore(appium): fix schema json path issues
- the path to the JSON schema is now correct: `lib/appium-config-schema.json`
- add `log-symbols` to devdeps for niceness. this is already a transitive dep
(where applicable)
- typed and documented some stuff
- added `fs.openFile` which resolves w/a `fsPromises.FileHandle` object. `fs.open` still resolves with a file descriptor. this is, I think, the only explicit conflict between the builtin `fs` module and ours. we should consider deprecating or removing our `fs.open` and using the `FileHandle`-returning impl, because it's a little easier to use. Once we do, we can also remove `fs.close()`, `fs.read()` and `fs.write()`, as those are redundant since they are methods of the `FileHandle` object returned from `fsPromises.open`. A cursory check in the `appium` org suggests that `open` and its ilk are not used except in some test cases.
- assign properties directly to the `fs` object, as this is easier for the type system to understand (rather than iterating over property names and assigning properties in the loop)
- `which` actually returns a `Promise` now and does not need promisification
- added note about future changes to `rmdir`
- fix some types, mocks
- rename `HASHFILE_RELATIVE_PATH` to `PKG_HASHFILE_RELATIVE_PATH`; hopefully that's less opaque
- fixed broken junk in `commands-yml/parse.js` which will conflict with #16459 so idk
- `mkdirp` module is now deprecated and re-exports `fs.mkdirp`, which is new
- fix some types in `fs` module
- fix `walkDir` which did not actually resolve to `null` ever
- Altered strategy for determining `APPIUM_HOME` by using `npm ls` and reading `package.json` if that fails
- rename `isLocalAppiumInstalled` to `hasAppiumDependency` in `env` module
- no longer care about `peerDependencies`; we can add this later if necessary
- consume `pkg-dir` module
- remove `read-pkg-up`
- move `npm` module into `support` (which is the `feat` here, I guess)
- the `NPM` class no longer has internal state, so doesn't really need to be a class, but I didn't want to bother with that refactor. the module exports the class `NPM` and an instance `npm`. creating new instances of `NPM` are useless, but there's no guard against it.
- method signatures in `NPM` class have changed to generally accept a `cwd` string. further, non-optional options have been moved out of an optional options object. 😄
feat(appium): remove NPM module (it went into @appium/support)
- Probably not a breaking change, but it certainly breaks a few things in `appium`, which I addressed.
- Updated for new `NPM` api
- fixed sandboxing issue in `cli-specs`
- update `mocks`
- `env` module exposes functions for determining `APPIUM_HOME`, where various files should live, etc., by examining the runtime environment
- fixed some functions in `fs` which unnecessarily used `this` (which prevented dereferencing)
- added `walk()` to `fs` which just exposes `klaw` (since `walkDir` has very limited behavior). this enables `walk()` to be used as an async iterator, which is handy.
- fixed a bunch of types in the `util` module. involved some actual code changes, but the original code only worked by chance.
- removed `strict` option from `util.coerceVersion()`, since the option isn't actually used anywhere. this should be considered a breaking change if some module somewhere _else_ uses it and we consider it a public API. I'm pretty sure that's not the case, so.
Jonathan Lipps