Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Planar detector tests #120

Merged
merged 9 commits into from Jun 7, 2019

Conversation

Projects
None yet
2 participants
@paullewis
Copy link
Collaborator

commented Jun 4, 2019

No description provided.

@paullewis paullewis requested a review from mmocny Jun 4, 2019

@paullewis

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 4, 2019

Moar tests. Sorry. (Not sorry.)

@mmocny

mmocny approved these changes Jun 4, 2019

Copy link
Collaborator

left a comment

Is this an alternative to PR #118 ?


it('adds, removes, and detects images', async () => {
const data = await loadDataFile();
const src = { id: 'Lighthouse', media: [] };

This comment has been minimized.

Copy link
@mmocny

mmocny Jun 4, 2019

Collaborator

This looks like the DetectableImage interface... but if media is empty, then I assume it's not actually used in addDetectionTarget? This makes for a confusing API. Should addDetectionTarget only accept image ID if it directly accepts image data?

(Either way, this seems like deep in the bowels of the detector code, so any API is fine as long as it makes sense to you.. ;)

paullewis added some commits Jun 5, 2019

Merge branch 'planar-detector-tests' of github.com:GoogleChromeLabs/p…
…erception-toolkit into planar-detector-tests
@paullewis

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 5, 2019

Is this an alternative to PR #118 ?

No, the first one dealt with the the nuts and bolts of the wasm (with a little JS glue), and this is more from the PT side where we expose the actual API that we use, and which ultimately goes down to the wasm stuff in 118.

paullewis added some commits Jun 5, 2019

@paullewis

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 7, 2019

Merging because of the flaky test outlined in #124

@paullewis paullewis merged commit cb5258a into master Jun 7, 2019

3 of 5 checks passed

Travis CI - Branch Build Failed
Details
Travis CI - Pull Request Build Failed
Details
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.