- 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`
## BREAKING CHANGES
(potentially, anyway)
1. Installation of extensions via `appium` now treats `$APPIUM_HOME` _mostly_ like a project containing a `node_modules` subfolder, with extensions under that. Previously, each extension would have its own folder, with a `node_modules` and empty `package.json`, and finally the extension. The intent was to inhibit deduping and conflicts with any `package.json` files in ancestors of `$APPIUM_HOME`. I showed this was unnecessary because the same thing can be accomplished via the flag `--global-style` for `npm install`. Ultimately, this simplifies the directory structure of `$APPIUM_HOME`.
2. `$APPIUM_HOME/extensions.yaml` now lives at `$APPIUM_HOME/node_modules/.cache/appium/extensions.yaml`. The `node_modules/.cache/<pkg-name>` convention is fairly common for packages having temporary files which need to persist while a module is installed (no more, no less).
3. If the _current working directory_ contains a `package.json` which has `appium` as any sort of (non-`file:`) dependency _or_ `$CWD/node_modules/appium` is a resolvable module, `APPIUM_HOME` defaults to `cwd`.
4. "Install" and "link" lockfiles (see `lib/cli/npm`) have also moved from `$APPIUM_HOME` to `$APPIUM_HOME/node_modules/.cache/appium`.
5. The `runExtensionCommand()` function in the `cli/extension` module no longer accepts a `type` parameter, which can be derived from the `configObject` parameter anyway (redundant)
6. Loading of extensions (i.e. reading `extensions.yaml`) has been moved into its own function, `loadExtensions()` in `extension/index.js`.
7. `getActivePlugins` and `getActiveDrivers` have been moved here from `lib/main`. Changed the signatures of these, since they don't actually need the entire `ParsedArgs` object.
8. `ext-config-io` module, which handles reading/writing `extensions.yaml` has been moved to `extension/manifest.js`.
9. The `installPath` property in `extensions.yaml` has been removed and is unneeded due to 1.
10. `read()` has been removed from `extension-config` and its subclasses.
11. Moved `ReadonlyMap` from `utils` to `schema/schema` module. Probably so I didn't have to stub it in a test.
12. Moved `rootDir` from `utils` to `config`.
13. Removed `drivers` module; moved `findMatchingDriver` to `DriverConfig#findMatchingDriver`. Removed `AppiumDriver#_findMatchingDriver`; calls `DriverConfig#findMatchingDriver` directly instead.
14. Removed `plugins` module; its contents now live in `constants`.
15. Dynamic determination of `APPIUM_HOME` now happens in the `env.resolveAppiumHome()` method of `@appium/support` (introduced in a previous commit). It should not happen anywhere else, because it will be wrong.
16. Because the `lib` dir was becoming rather unwieldy, I moved modules associated with extension loading and config into an `extension` subdir
17. `getParser()` in `cli/parser` is no longer async; the async stuff it was doing moved into `loadExtensions()`. `getParser()` can be used without calling `loadExtensions()` first (no extensions will be loaded)
## Summary
This enables installation of extensions via `npm`; e.g., `npm install appium-xcuitest-driver -D`. Because `appium` usually manages the `extensions.yaml` (the "manifest"), this is not possible when installing modules via other means. To handle this situation, if we have an `appium`-dep-containing `package.json` in `APPIUM_HOME`, a hash is taken of the `package.json`. This hash is checked at runtime; if it has changed, `appium` will scan `node_modules` (_not_ `package.json`; we only care about extensions that are _actually installed_, which is different from how we handle an `appium` dep) for extensions, and the hash will be updated. Extensions are defined as "modules having a `package.json` which contains an `appium` field which is an object of a particular shape". When these are found, they are appended to `extensions.yaml`. In theory, this allows a user to use `appium` _or_ `npm` (at their convenience) to manage extensions.
See the "when appium is a dependency" suite in `test/cli/cli-e2e-specs.js` for more context.
## Notes
- Added a `constants` module, which provides a centralized place for various constants. This makes finding constants less cognitively demanding. 🙂 You may not like this; if you don't, the constants can be moved to more appropriate places.
- Added `extension/package-changed` module which is used to determine if a `package.json` has changed
- Many types are added/changed throughout. See especially `extension/manifest` for many type definitions involving the shape of `extensions.yaml` and the `appium` prop of extensions' `package.json`.
- Reorganized some tests based on structure of `lib`.
- Split `extension-config-specs.js` into `driver-config-specs.js` and `plugin-config-specs.js`.
- Many refactors in now-fully-typed `cli-e2e-specs`; introduced some more helpers in `test/cli/cli-helpers.js` and used these. Makes calling out to the `appium` executable from E2E tests a little easier.
- Introduced `test/extension/mocks` for DRY mocks, since many of the unit tests here now use the same ones.
- Consolidated fixtures into `test/fixtures`. Fixtures can always be found via the `resolveFixture()` method of `test/helpers.js`.
## TODO (before this lands)
- Instead of using `$CWD` for `APPIUM_HOME` if `appium` is a dep, we probably want to use `find-pkg-up` or something. This should be okay, since if we find a `package.json` and `appium` _isn't_ a dep, we will just be using the default `APPIUM_HOME`.
- We _may_ also need to search `package.json` for known extensions, since it's possible to use them standalone. However, I'm not sure if this use case needs `APPIUM_HOME` at all?
chore: vscode stuff
- change default build task to `dev`, which just transpiles in the background
- updated `test - current file` launch config for better debugging
chore(appium): peer review refactors
- The old class name of `Manifest` used to be `ExtConfigIO`, but I forgot to update these variable names. `io` is now `manifest`, wherever used
- Created a more clear separation between the `create()` and `getInstance()` static methods of `DriverConfig`/`PluginConfig`. `create()` should _only_ be used for instantiation, and `getInstance()` should _only_ be used if we have already instantiated the object with an associated `Manifest`. `create()` now _throws_ if the `Manifest` already has an association in place. The reason this is split into two functions instead of just using `_.memoize()` is because of the options in the second parameter (`extData`, `logFn`), which would require the _same configuration object_ in order to return the already-created instance. This is not a problem in `Manifest`, where the factory only accepts a single parameter.
- The `_instances` static method of `PluginConfig`/`DriverConfig` is now a `WeakMap`, keyed on a `Manifest` instance instead of the `appiumHome` string. This makes the relationship between them more explicit.
- Fixed some formatting
- Fixed type issues in schema tests, `grid-register` and its tests
- Made the constructor of `Manifest` private, created a memoized static method `getInstance()`; removed `getManifestInstance()` export.
- Refactored `let`-destructuring to the fancy new syntax I learned from @mykola-mokhnach
- Explained the meaning of `CONFIG_DATA_DRIVER_KEY` and `CONFIG_DATA_PLUGIN_KEY` in `manifest` module
- Replaced unnecessary use of `null` with `undefined` in `Manifest` class
- Removed runtime type check from `packageDidChange()`
- Had some fun renaming `ReadonlyMap` in `schema` module
- `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.
- Upgrade TS to `next` tag. It's "safe" to use using bleeding-edge TS here since we don't actually use it to compile anything
- Add a bunch of missing types: `find-root`, `klaw`, `lockfile`, `mv`, `ncp`, `rimraf`
`--show-config` was not able to distinguish between defaults and values provided on the CLI. now it does. `--show-config` now displays (in this order):
1. any settings from the defaults (across Appium and any extensions)
2. any settings specified in config files
3. any settings specified on the CLI
4. the final merged configuration
Ref: #16340
* feat(appium): rename --show-config to --show-build-info
* feat(appium): add --show-config
This adds a new CLI flag: `--show-config`. When called, this will dump the values of a) arguments to the `init()` function of `main.js` _or_ args passed thru CLI, b) any config pulled from a config file, and c) the defaults as defined in schemas.
Each section is separated, and `undefined` values are omitted.
Instead of a JSON blob, the output just uses `console.dir()`.
To make this work, some refactors were needed:
- Use of `store_true` "action" for `argparse` in are now `store_const` with `const: true` because `store_true` sets the value to `false` if the flag is not present. This essentially assigns the default value of the flag (`false`) before it should; default values must come from the schemas.
- Along those lines, removed default values from the "extra" boolean args in `args.js`
- The CLI parser now ignores `undefined` values of arguments
Other:
- Created some types for extension classes and added the `server` prop to `AppiumDriver`.
- Enabled TS checks on `lib/config.js` (where `showConfig()` lives) and `test/config-specs.js`
- Removed unnecessary call to `validate()` in `preflightChecks()`; CLI arguments are validated individually (defined in `lib/schema/cli-args.js`) and the config file is validated separately (defined in `lib/config-file.js`).
- Add more types to `main.js`; rewrite `getServerUpdaters` in lodashese for fun (which I am happy to revert)
- Fix missing type aliases in `test/cli/cli-helpers.js`
- Misc types/refactors in `config-specs.js`
- Update parser tests to reflect that `undefined` values and empty objects are eliminated from the parsed arguments
Notes:
- The `showBuildInfo()` test in `config-specs.js` (which is ostensibly a unit test) forks `git` and also seems to require network connectivity, at least on my box. This stuff should be stubbed out or moved into an E2E test.
Resolves#15672
* chore(appium): remove unused options in `toParserArgs()`
This PR moves the FOSSA check from the Node workflow to its own. It won't run if the API key secret is not present (which would be true when running PRs from forks).
In addition, the check
1. will only run on changes to any `package.json` in the monorepo
2. runs `fossa test` to fail upon failure (previously it would only fail upon _error_)
Some theoretical schemas are not in any sort of shape Appium can use, so we now catch them as soon as possible (at the registration phase).
- adds type guard
- adds
Extensions were not correctly receiving the default values as specified in their schemas. For example, in `xcuitest` driver, the `wdaLocalPort` option has a default value of `8100`, but this value would never make it to the driver.
The root cause was that `getDefaultsForSchema()` returns a _flattened_ object (which is useful for displaying to users as part of `getNonDefaultServerArgs()`), but by the time defaults should get assigned to parsed arguments in `main.js`, the parsed arguments object has already been expanded to a deeply-nested, non-flattened object. So `"driver.driverName.argName": "value"` would be set in the args, but not `{"driver": {"driverName": {"argName": "value"}}}`. Furthermore, the properties of `cliArgs` are not prefixed with `<extType>.<extName>.`, but are just the arg name itself (which makes sense).
To address this, two main changes:
1. `getDefaultsForSchema(flatten = true)` (renamed from `getDefaultsFromSchema()`) now accepts a `flatten` flag, which will return a flat object using `<extType>.<extName>.`-style prefixes. This is useful (and less computationally intensive) for comparing defaults across the whole of Appium and its extensions. This is _not_ useful when merging defaults into the parsed arguments object, however, as that object will be deeply-nested. If `flatten` is `false`, a deeply-nested object will be returned, which is consumed by the `main` module when post-processing parsed CLI arguments and config file values.
2. Added `getDefaultsForExtension(extType, extName)` which acts like `getDefaultsForSchema(flatten = true)` _but_ only returns defaults for a particular extension _and_ strips the leading prefixes. This is used by `assignCliArgsToExtension` in the `AppiumDriver` class.
Other changes:
1. `ArgSpec` now keeps a `rawDest` property which is like `dest` w/o the prefixes.
2. Make more types
3. Re-organize `driver-specs` unit test somewhat, and update to reflect changes.
Christopher Hiller