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

Make sure TypedArrays are supported everywhere #3215

Closed
alexcjohnson opened this issue Nov 5, 2018 · 3 comments
Closed

Make sure TypedArrays are supported everywhere #3215

alexcjohnson opened this issue Nov 5, 2018 · 3 comments

Comments

@alexcjohnson
Copy link
Collaborator

Discussion from #3210:

(@alexcjohnson) I'll fix (and test) this specific bug, but @etpinard I'm not sure what would make a good generic test of typed array support - in principle we could rerun the entire image test suite, combing through data (and layout for numeric arrays and converting them to typed arrays first... that seems pretty heavy but how else could we catch silent bugs like this?

(@etpinard) That does seems pretty heavy, unless we can add two more CircleCI test containers 💰

But maybe we could try to run all our mocks with typed arrays once locally and then lock failing cases (if any 😏 ) with jasmine tests.

And how do we catch these problems in new features then? I guess if there's a simple command to run all of our mocks with TypedArrays, we could make this part of the pre-publish checklist?

@jonmmease
Copy link
Contributor

I just opened #3339, and by my count this is the 6th bug so far that would have been caught if all of the mocks were run with typed arrays.

Full list:

So it seems to me that time/resources spent on preventing this class of issues in testing would be pretty worthwhile.

@jonmmease
Copy link
Contributor

And #3595

etpinard referenced this issue Apr 26, 2019
... after transform _module.calc loop

- that way filter transforms that remove all data coordinates
  don't result in errors
- of all the mocks listed in mock_lists.js, only 'scattercarpet' and
  'world-cals' still error out (wip)
@gvwilson
Copy link
Contributor

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants