Skip to content
This repository has been archived by the owner on Apr 19, 2022. It is now read-only.

feat(execute-driver): add execute driver script plugin #73

Merged
merged 3 commits into from
Jan 21, 2022

Conversation

jlipps
Copy link
Contributor

@jlipps jlipps commented Nov 17, 2021

  • this plugin is being ported from base driver
  • also: use process.env.APPIUM_HOME for tests now since it's an env var
  • also: clean up tsconfig stuff a bit
  • also: use @appium/fake-driver instead of appium-fake-driver
  • also: add clean:tsc script to remove build dirs

@jlipps
Copy link
Contributor Author

jlipps commented Nov 19, 2021

CI will fail until appium/appium#16137 is merged and published

package.json Outdated Show resolved Hide resolved
packages/execute-driver/LICENSE Show resolved Hide resolved
packages/execute-driver/lib/execute-child.js Outdated Show resolved Hide resolved
packages/execute-driver/lib/execute-child.js Outdated Show resolved Hide resolved
packages/execute-driver/lib/execute-child.js Outdated Show resolved Hide resolved
packages/execute-driver/test/e2e/plugin-e2e-specs.js Outdated Show resolved Hide resolved
packages/execute-driver/test/e2e/plugin-e2e-specs.js Outdated Show resolved Hide resolved
packages/images/test/e2e/plugin-e2e-specs.js Show resolved Hide resolved
@boneskull
Copy link
Contributor

boneskull commented Nov 20, 2021

perhaps I should do the thing I did to appium on this monorepo, to avoid the lockfile apocalypse

@mykola-mokhnach
Copy link

Linking appium/appium#15973

@jlipps jlipps force-pushed the jlipps/execute-driver branch from 16004cf to 89b507a Compare November 23, 2021 03:11
@jlipps
Copy link
Contributor Author

jlipps commented Nov 23, 2021

OK @mykola-mokhnach @boneskull made a bunch of updates here, including using vm2 in place of vm. Someone should double check this though. It works but that doesn't mean I did it right.

@jlipps jlipps force-pushed the jlipps/execute-driver branch from 89b507a to 143b7aa Compare November 23, 2021 03:29
package.json Show resolved Hide resolved
@jlipps jlipps force-pushed the jlipps/execute-driver branch from 143b7aa to 7fe8631 Compare November 23, 2021 15:35
package.json Show resolved Hide resolved
(this plugin is being ported from base driver)
- also: use process.env.APPIUM_HOME for tests now since it's an env var
- also: clean up tsconfig stuff a bit
- also: use @appium/fake-driver instead of appium-fake-driver
- also: add clean:tsc script to remove build dirs
@jlipps jlipps force-pushed the jlipps/execute-driver branch 2 times, most recently from 4c221ff to be3a649 Compare December 3, 2021 19:12
@jlipps
Copy link
Contributor Author

jlipps commented Dec 3, 2021

well, CI is now broken because of opencv4nodejs. i've attempted to debug the issue. i'm guessing that an NPM update changed node modules layout and a package internal to opencv4nodejs is relying on a certain directory structure. i can't figure out how to see full logs from the workflow (the npm output isn't showing the logging that should be coming from opencv4nodejs's install scripts). that might help. but sadly opencv4nodejs is not maintained so it may be impossible to fix. ugh.

@mykola-mokhnach
Copy link

@jlipps Could you please try with the fork of opencv4nodejs mentioned by @UrielCh in justadudewhohacks/opencv4nodejs#762 (comment) ?

@jlipps jlipps force-pushed the jlipps/execute-driver branch from be3a649 to 1fda9c7 Compare January 11, 2022 00:10
@jlipps
Copy link
Contributor Author

jlipps commented Jan 11, 2022

ok, giving it a go

@jlipps
Copy link
Contributor Author

jlipps commented Jan 11, 2022

nope, it's actually imported from appium-support. i wonder if we should move it to the images plugin? anyway, https://github.com/appium/appium/pull/16327/files

@jlipps jlipps force-pushed the jlipps/execute-driver branch from 1fda9c7 to db9976e Compare January 11, 2022 21:08
@jlipps
Copy link
Contributor Author

jlipps commented Jan 11, 2022

ok pulled in appium support changes, let's see what happens.

@jlipps jlipps force-pushed the jlipps/execute-driver branch from db9976e to 787f748 Compare January 11, 2022 23:16
@jlipps
Copy link
Contributor Author

jlipps commented Jan 11, 2022

@mykola-mokhnach looks like this package doesn't work. It also doesn't install on my machine even with the m1-specific flags enabled. https://github.com/appium/appium-plugins/runs/4782669773?check_suite_focus=true

@mykola-mokhnach
Copy link

Thanks for trying it @jlipps
Perhaps we could report the issue log at https://github.com/appium/appium-plugins/runs/4782669773?check_suite_focus=true#step:9:3393 to the forked repository issue tracker and ask its author about how we could resolve it. Let see how responsive they are.

@jlipps
Copy link
Contributor Author

jlipps commented Jan 12, 2022

The forked repo doesn't appear to have the issue tracker enabled. @UrielCh are you at all open to helping us troubleshoot this opencv4nodejs installation in GitHub Actions CI? (I get the same error on my local machine). Have tried with opencv version set to latest 3.x as well as latest 4.x.

@mykola-mokhnach
Copy link

I assume that compilation error happens because of https://answers.opencv.org/question/212650/opencv-sift-surf/

@jlipps
Copy link
Contributor Author

jlipps commented Jan 14, 2022

Running with OPENCV4NODEJS_AUTOBUILD_OPENCV_VERSION=4.5.5 OPENCV_BUILD_ROOT=~/opencv npx build-opencv --flag="-DCMAKE_SYSTEM_PROCESSOR=arm64 -DCMAKE_OSX_ARCHITECTURES=arm64 -DCMAKE_OPENCV_ENABLE_NONFREE=ON" build gives the same error. What do you run on your machine to get things to build?

@mykola-mokhnach
Copy link

ofc, will try it on the weekend

@mykola-mokhnach
Copy link

mykola-mokhnach commented Jan 15, 2022

@jlipps I have managed it to work on my Intel mac.
The steps I've done:

  • Install node@14 from brew and set it as the default one. I tried with node@16 first and was not able to compile opencv sources
  • Checkout https://github.com/UrielCh/opencv4nodejs.git
  • Make sure tsc is installed (npm i -g typescript)
  • Create a folder for opencv build: mkdir /Users/user/opencv
  • Run the following command in the root of the previously checked out opencv4nodejs module: OPENCV_BUILD_ROOT=/Users/user/opencv node node_modules/@u4/opencv-build/bin/main.js --version 4.5.4 install (The build of 4.5.5 has failed on my machine due to some zlib dependency). I assume for arm-based macs some additional command line parameters should be provided. See https://github.com/UrielCh/opencv4nodejs/blob/fad9f016380d8207dc9b86ee523cfe95cbad9ba5/package.json#L39
  • Wait until OpenCV building is completed
  • Install the previously built libraries: OPENCV_BUILD_ROOT=~/opencv OPENCV4NODEJS_DISABLE_AUTOBUILD=1 OPENCV_INCLUDE_DIR=/Users/user/opencv/opencv-4.5.4/build/include OPENCV_LIB_DIR=/Users/user/opencv/opencv-4.5.4/build/lib OPENCV_BIN_DIR=/Users/user/opencv/opencv-4.5.4/build/bin npm run do-install build
  • Link the @u4/opencv4nodejs library to appium-support. I've just created a simple symlink in my test: ln -s ~/code/opencv4nodejs ~/code/appium-support/node_modules/@u4
  • Now it is possible to run a e2e test. Do not forget to provide the necessary env variables. I believe this stuff could also be added directly into package.json of appium-support to skip env preparation, but I did not try such option. The resulting command to run tests would be: OPENCV_BUILD_ROOT=/Users/user/opencv OPENCV_INCLUDE_DIR=/Users/user/opencv/opencv-4.5.4/build/include OPENCV_LIB_DIR=/Users/user/opencv/opencv-4.5.4/build/lib OPENCV_BIN_DIR=/Users/user/opencv/opencv-4.5.4/build/bin _FORCE_LOGS=1 gulp e2e-test

@mykola-mokhnach
Copy link

mykola-mokhnach commented Jan 15, 2022

Btw many thanks to @KeitelDOG for the detailed description of the building process. It helped me a lot.

@jlipps
Copy link
Contributor Author

jlipps commented Jan 21, 2022

OK, this PR now includes a change where the images plugin depends on @appium/opencv and no longer requires opencv4nodejs to be installed in CI. let's see how it goes.

@jlipps jlipps force-pushed the jlipps/execute-driver branch from a298559 to 37cc12e Compare January 21, 2022 20:25
@jlipps
Copy link
Contributor Author

jlipps commented Jan 21, 2022

yay, build passed! we can finally get the execute driver plugin merged, and publish an images plugin with no dependency on opencv4nodejs!!!

@jlipps jlipps merged commit ef65ac3 into master Jan 21, 2022
@jlipps jlipps deleted the jlipps/execute-driver branch January 21, 2022 20:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants