modernized the TESTING architecture of the codebase { GSOC} by endurance21 · Pull Request #541 · processing/p5.js-sound

endurance21 · 2020-08-24T03:31:04Z

MORE CLEANER AND INTUITIVE TESTING ARCHITECTURE

getting rid of requirejs .finally !
used es6 imports
used npm to download "chai" and "mocha" and removed the cached version in test/testDeps/chai.js , test/testDeps/mocha.js
thus reducing the size of the codebase !
can be further linked to karma.js for headless testing ! { future goals}
can be further used for integrating in CI environment !

endurance21 · 2020-08-24T03:32:35Z

@therewasaguy @kyle1james
this one for the testing architecture !

for now i have removed sinon.js ,from test/testDeps/sinon.js , i would like to hear some suggestions about its requirements for unit testing now and in future!

therewasaguy

thus reducing the size of the codebase

Seriously! This is awesome

Aside from a couple minor comments, the only real issue is that this isn't working on Firefox (79.0). Getting this error

Uncaught DOMException: AudioWorkletNode constructor: Unknown AudioWorklet name 'sound-file-processor'

I suspect it has something to do with the way we're importing the audio worklet modules inline with raw-loader here

p5.js-sound/src/audioWorklet/index.js

Lines 2 to 6 in 91d8b34

    
           const moduleSources = [ 
        
             require('raw-loader!./recorderProcessor').default, 
        
             require('raw-loader!./soundFileProcessor').default, 
        
             require('raw-loader!./amplitudeProcessor').default, 
        
           ];

It's possible that raw-loader doesn't work well with firefox <script ... type="module">

Hoping for a quick fix, I tried upgrading the raw-loader dependency to 4.0.1 but that doesn't seem to resolve the issue...

UPDATE

seems like raw-loader is working as expected, it's just loading the modules after we need them

p5.js-sound/src/audioWorklet/index.js

Lines 21 to 34 in 91d8b34

    
           p5.prototype.registerMethod('init', function () { 
        
             if (initializedAudioWorklets) return; 
        
             // ensure that a preload function exists so that p5 will wait for preloads to finish 
        
             if (!this.preload && !window.preload) { 
        
               this.preload = function () {}; 
        
             } 
        
             // use p5's preload system to load necessary AudioWorklet modules before setup() 
        
             this._incrementPreload(); 
        
             const onWorkletModulesLoad = function () { 
        
               initializedAudioWorklets = true; 
        
               this._decrementPreload(); 
        
             }.bind(this); 
        
             loadAudioWorkletModules().then(onWorkletModulesLoad);

therewasaguy · 2020-08-24T05:04:15Z

alright, I think the issue with Firefox is actually an issue with how we've structured some of the tests. For example, I've commented out all of the import statements in test.js except for tests/p5.Master.js and that test runs just fine. Meanwhile tests/p5.SoundFile.js has a problem. The issue is that we're creating a p5.Amplitude (which needs an AudioWorklet module) before the p5 init method has completed, and for some reason Firefox is the only browser that hasn't registered the AudioWorklet modules by this point.

We can fix this by creating the amplitude as part of the test lifecycle, wrapping this

p5.js-sound/test/tests/p5.SoundFile.js

Line 10 in 91d8b34

var a = new p5.Amplitude();

in a before method

  var a;

  before(() => {
    a = new p5.Amplitude();
  });

…strict declaration

endurance21 · 2020-08-29T08:01:22Z

@therewasaguy added sinon.js for as it was used inside test/tests/p5.soundRecorder.js

alos removed
use strict keyword

endurance21 · 2020-09-30T05:48:46Z

@therewasaguy

this audioWorklet is not loading at the correct time of its access by p5.oscilllator and other processors .
and causing the problem .and mocha hooks is not enough to fix them i see.
This is the problem !

…e to ensure proper loading of AudioWorklet modules for * sounFile processor * Oscillator processor * Amplitude processor * test/index.js - then used dynamic import to import the tests after each of worklet processors have been loaded successfully

endurance21 · 2020-09-30T21:12:09Z

@therewasaguy i have solved the problem ,

actually, there was no way we could be able to know when all AudioWorkletProcessor will load finally so I have attached a new
promise to know when will all audioWorklet processors will be laoded finally and then used that promise to load the test !

the tests are running fine in firefox too !

and issue seems to be resolved !
have a look then we can move forward
!

endurance21 · 2020-12-27T20:41:43Z

@therewasaguy hey! it been long seeing this.
Lets have a good warm up start and we can move ahead for smth good coming year!

montoyamoraga

hi @endurance21 all of this looks great, thanks!
i'm not familiar with the changes you are doing,
and i don't know how to test it, so i will let someone else merge it :)

oshoham · 2021-01-02T20:00:29Z

@therewasaguy i have solved the problem ,

actually, there was no way we could be able to know when all AudioWorkletProcessor will load finally so I have attached a new
promise to know when will all audioWorklet processors will be laoded finally and then used that promise to load the test !

the tests are running fine in firefox too !

and issue seems to be resolved !
have a look then we can move forward
!

Hey @endurance21, the intention of the AudioWorkletProcessor loading code that I wrote was that all processor modules would be available when p5's setup function runs.

At the time, there was no way to ask p5 to wait for an arbitrary promise before running setup, but I was able to use p5's internal preloading hook to load the modules, which is guaranteed to finish before setup.

Is it possible for you to just use the p5 setup function in your tests instead of adding a new promise that isn't used anywhere else in the codebase?

Alternatively, if p5 has better support for promises these days, maybe you could replace the preloading code entirely?

endurance21 · 2021-01-02T21:03:11Z

@oshoham thanks a ton for the review, I need to give it a shot by using preload hook and see if that helps.

…tance mode

endurance21 · 2021-01-05T00:04:05Z

@oshoham it worked that way! thanks again!

endurance21 · 2021-01-05T00:06:10Z

@therewasaguy i have tested on firefox and chrome too, tests are running smooth now!

therewasaguy

awesome work @endurance21 !

therewasaguy · 2021-02-03T17:57:51Z

+    <link rel="stylesheet" href="../node_modules/mocha/mocha.css" />
+    <script src="../node_modules/mocha/mocha.js"></script>
+    <script src="../node_modules/chai/chai.js"></script>
+    <script src="../node_modules/sinon/pkg/sinon.js"></script>


we're dependent on the file structure of node_modules here, just an observation, but I do think this is better than what we had before !

earlier those files are stored with fixed version in codebase , now we use npm install and it downloads from npm for every codebase setup!
that way we just reduced file size of our codebase!

modernized the TESTING architecture of the codebase

cb61b04

endurance21 requested review from LilaShiba and therewasaguy August 24, 2020 03:32

therewasaguy reviewed Aug 24, 2020

View reviewed changes

Comment thread test/tests/p5.Amplitude.js Outdated

Comment thread test/tests/p5.Amplitude.js

Comment thread test/index.html Outdated

xadded sinon.js support for soundRecorder.js testing and removed use …

5ef1d6f

…strict declaration

endurance21 requested a review from therewasaguy August 29, 2020 08:01

therewasaguy reviewed Aug 29, 2020

View reviewed changes

Comment thread test/tests/p5.Oscillator.js

therewasaguy reviewed Aug 29, 2020

View reviewed changes

Comment thread test/tests/p5.SoundFile.js

endurance21 requested a review from therewasaguy September 30, 2020 15:13

Divyanshu Raj and others added 2 commits October 1, 2020 02:31

Merge branch 'master' into testingFinal

a42d8cf

endurance21 requested review from montoyamoraga and oshoham January 2, 2021 13:29

montoyamoraga reviewed Jan 2, 2021

View reviewed changes

oshoham reviewed Jan 2, 2021

View reviewed changes

Comment thread src/audioWorklet/index.js Outdated

oshoham reviewed Jan 2, 2021

View reviewed changes

Comment thread src/audioWorklet/index.js

endurance21 mentioned this pull request Jan 4, 2021

fixed issue with p5.Score that prevented 'parts' from being passed as… #543

Merged

solved loading of audioWorklet processors in testing by use of p5 ins…

a736d8a

…tance mode

fixed audioworklet import

fe1395d

endurance21 commented Jan 5, 2021

View reviewed changes

therewasaguy approved these changes Feb 3, 2021

View reviewed changes

Merge branch 'master' into testingFinal

66855ea

endurance21 merged commit 2fd6a90 into processing:master Feb 4, 2021

	const moduleSources = [
	require('raw-loader!./recorderProcessor').default,
	require('raw-loader!./soundFileProcessor').default,
	require('raw-loader!./amplitudeProcessor').default,
	];

	p5.prototype.registerMethod('init', function () {
	if (initializedAudioWorklets) return;
	// ensure that a preload function exists so that p5 will wait for preloads to finish
	if (!this.preload && !window.preload) {
	this.preload = function () {};
	}

	// use p5's preload system to load necessary AudioWorklet modules before setup()
	this._incrementPreload();
	const onWorkletModulesLoad = function () {
	initializedAudioWorklets = true;
	this._decrementPreload();
	}.bind(this);
	loadAudioWorkletModules().then(onWorkletModulesLoad);

Conversation

endurance21 commented Aug 24, 2020

MORE CLEANER AND INTUITIVE TESTING ARCHITECTURE

Uh oh!

endurance21 commented Aug 24, 2020

Uh oh!

therewasaguy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

UPDATE

Uh oh!

Uh oh!

Uh oh!

Uh oh!

therewasaguy commented Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

endurance21 commented Aug 29, 2020

Uh oh!

Uh oh!

Uh oh!

endurance21 commented Sep 30, 2020

Uh oh!

endurance21 commented Sep 30, 2020

Uh oh!

endurance21 commented Dec 27, 2020

Uh oh!

montoyamoraga left a comment

Choose a reason for hiding this comment

Uh oh!

oshoham commented Jan 2, 2021

Uh oh!

Uh oh!

Uh oh!

endurance21 commented Jan 2, 2021

Uh oh!

endurance21 commented Jan 5, 2021

Uh oh!

endurance21 commented Jan 5, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

therewasaguy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

therewasaguy Feb 3, 2021

Choose a reason for hiding this comment

Uh oh!

endurance21 Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

therewasaguy left a comment •

edited

Loading

therewasaguy commented Aug 24, 2020 •

edited

Loading