Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

Require from global if opencv4nodejs is not found locally #105

Merged
merged 5 commits into from
Aug 26, 2019

Conversation

imurchie
Copy link
Contributor

@imurchie imurchie commented Mar 1, 2019

Node does not work as the code assumes it does. If a module is not found locally the system does not, by some magic, look at the globally installed modules (see nodejs/node#3865).

I tried to find a published package that does this but didn't find anything I was particularly confident of. And the actual code is pretty simple.

This PR adds a little utility function (currently not exported, since we don't have any need for it anywhere else) to find the global package and load it.

This ought to finally solve appium/appium#11865

lib/image-util.js Outdated Show resolved Hide resolved
lib/image-util.js Outdated Show resolved Hide resolved
lib/image-util.js Outdated Show resolved Hide resolved
Copy link
Member

@jlipps jlipps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great idea, @imurchie, thanks. this has been bugging me. I always resorted to adding the global dir to my NODE_PATH in my zshrc, but that's an extra step for everyone.

i would consider exporting this, since we already need it for test-ai-classifier as well, for those who are using that feature.

changes LGTM but i agree with @mykola-mokhnach about the error cases.

@@ -66,7 +66,7 @@ describe('image-util', function () {
});

describe('getImagesMatches', function () {
it('should calculate the number of matches between two images', async function () {
it.only('should calculate the number of matches between two images', async function () { // eslint-disable-line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this only intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Trying to get Windows working on Travis.

@imurchie
Copy link
Contributor Author

imurchie commented Mar 1, 2019

Ok. Once I get it stable and correct, I will make it exported.

}

// find the npm global root
const cmd = `npm${isWindows() ? '.cmd' : ''}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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.

None yet

4 participants