1044224 - [Messages][Tests] Generate assets on the fly instead of loading it with XHR

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Oleg Zasypkin [:azasypkin]]

The purpose of this bug is to try to generate some assets\blobs used in SMS unit tests on the fly (ex. using canvas and dataURI) to avoid loading it with XHR that causes intermittent timeouts in unit tests sometime. See [1] as an example.

[1] https://github.com/mozilla-b2g/gaia/blob/dd42afb7d6d66a6c2bd999692fec6b6d553d23de/apps/sms/test/unit/utils_test.js#L1116

Comment 1

[Oleg Zasypkin [:azasypkin]]

Attachment: GitHub pull request URL

Hey Julien,

Here is what I have in mind for this bug (for utils tests only for now). On my local machine loading of all resources in utils took ~1000ms, with generating on the fly only ~30ms.

If you're ok with this approach I can use it for the rest of tests that are dependent on image resources.

Thanks!

Comment 2

[Julien Wajsberg [:julienw]]

Comment on attachment 8463242 [details] [review]
GitHub pull request URL

yep, looks good

I'm only concerned that using images with a single color is not really like what happens in the real life but looks like that with the right "limit" value we can still test the various cases.

Maybe using a pixelated generated image (each odd pixel has the same color, each even pixel as another color) would be closer to the images we get in reality?

Comment 3

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #2)
> Comment on attachment 8463242 [details] [review]
> GitHub pull request URL
> 
> yep, looks good
> 
> Maybe using a pixelated generated image (each odd pixel has the same color,
> each even pixel as another color) would be closer to the images we get in
> reality?

Yeah, that's easy to do, will make something like you suggest. Thanks!

Comment 4

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8463242 [details] [review]
GitHub pull request URL

Added AssetsHelper that is used in all tests that previously were loading resources with the raw XHR call + auto-generated image is filled with gradient instead of solid color now.

Comment 5

[Oleg Zasypkin [:azasypkin]]

Forgot to add link that shows how this auto generated gradient looks like:
http://jsfiddle.net/azasypkin/Sx7KM/

Comment 6

[Julien Wajsberg [:julienw]]

Comment on attachment 8463242 [details] [review]
GitHub pull request URL

several comments on github :)

thanks !

Comment 7

[Oleg Zasypkin [:azasypkin]]

Hi Yuren, 

I've been told that you may know answer to the following question :)

Is it possible to somehow exclude app's specific file/folder from the application package (application.zip)? 

We have a bunch of files required only if Gaia is run inside browser, so it would be nice to ignore such files when app is pushed to device.

Thanks!

Comment 8

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8463242 [details] [review]
GitHub pull request URL

(In reply to Julien Wajsberg [:julienw] from comment #6)
> Comment on attachment 8463242 [details] [review]
> GitHub pull request URL
> 
> several comments on github :)
> 
> thanks !

Fixed, please take a look again!

Thanks!

Comment 9

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Hi Oleg, the status for pushing profile to device is not available in app-specific build script, but you can know if this build is a production build.

put this script to apps/sms/build/build.js then desktop-only directory will be removed if use |make production| or |PRODUCTION=1 make|

> 'use strict';
> 
> /* global require, exports */
> 
> var utils = require('utils');
> 
> exports.execute = function(options) {
>   if (options.PRODUCTION === '1') {
>     var desktopOnlyDir = utils.getFile(options.STAGE_APP_DIR, 'js',
>       'desktop-only');
>     if (desktopOnlyDir.exists()) {
>       desktopOnlyDir.remove(true);
>     }
>   }
> };

Comment 10

[Julien Wajsberg [:julienw]]

Comment on attachment 8463242 [details] [review]
GitHub pull request URL

r=me once you add the asserts for the small blobs, and you change the image name.

Can you also file a separate bug for adding a build script to ignore the desktop-only directory in production?

Comment 11

[Julien Wajsberg [:julienw]]

(In reply to Julien Wajsberg [:julienw] from comment #10)
> Comment on attachment 8463242 [details] [review]
> GitHub pull request URL
> 
> r=me once you add the asserts for the small blobs, and you change the image
> name.

And obviously, wait for a green enough try ;)

Comment 12

[Oleg Zasypkin [:azasypkin]]

(In reply to Yuren [:yurenju] from comment #9)
> Hi Oleg, the status for pushing profile to device is not available in
> app-specific build script, but you can know if this build is a production
> build.
> 
> put this script to apps/sms/build/build.js then desktop-only directory will
> be removed if use |make production| or |PRODUCTION=1 make|

Thanks a lot Yuren!

(In reply to Julien Wajsberg [:julienw] from comment #10)
> Can you also file a separate bug for adding a build script to ignore the
> desktop-only directory in production?

Sure, here is the bug 1050416.

Comment 13

[Oleg Zasypkin [:azasypkin]]

(In reply to Julien Wajsberg [:julienw] from comment #11)
> (In reply to Julien Wajsberg [:julienw] from comment #10)
> > Comment on attachment 8463242 [details] [review]
> > GitHub pull request URL
> > 
> > r=me once you add the asserts for the small blobs, and you change the image
> > name.
> 
> And obviously, wait for a green enough try ;)

Added asserts for small images + changed name for the images you pointed out, and try is mostly green :) Thanks!

Master: https://github.com/mozilla-b2g/gaia/commit/d5f0a11f7815d1787e398b7da2feb915e2ba49e9