Hi Victoria/Joao,

Thank you for reviewing. The above function is probably the last solution
which I added.
The test cases in web/regression/javascript/sqleditor/execute_query_spec.js
calls the execute query function of pgAdmin4 sqleditor. The execution
disables/enables the buttons internally and is not done by the test cases.
So there is spy on prop function (jqueryPropSpy = spyOn($.fn, 'prop')) of
jQuery. The problem is, the spy jQuery object (in function
findJQueryCallWithSelector) returned seems to have very less information,
not sure why. I had tried using css selector for id, tried extracting id
directly, class, but none of them were present in the object.

I can try adding data-test-selector to the HTML5 tag, if it works then will
send the updated patch.


Thanks and Regards,
Aditya Toshniwal
Software Engineer | EnterpriseDB Software Solutions | Pune
"Don't Complain about Heat, Plant a tree"

On Wed, May 23, 2018 at 7:41 PM, Joao De Almeida Pereira <
jdealmeidapere...@pivotal.io> wrote:

> HI Aditya,
>
> Good job porting the app from the ancient version of JQuery to the latest
> build :D
>
> The patch in general looks good, and the patches-bot says all tests pass.
>
> The only thing that we would like to point out is this piece of code:
>
> /* jQuery has removed selector property in version 3.x
>  * To allow JS test cases to run, the below function is
>  * used to set the selector property
>  */
> function setPropAndSelector(selector, propName, propVal) {
>   let tmpObj = $(selector);
>   tmpObj['selector'] = selector;
>   tmpObj.prop(propName,propVal);
>   return tmpObj;
> }
>
> As tests are also part of the code we believe that when there is a change
> that affect testing we should change the tests to work accordingly. So our
> suggestion is, instead of creating this function, why don’t we use a CSS
> selector in the tests to do the clicking or even use the HTML5 tag
> data-test-selector?
> This way the tests will also be coherent with the upgrade from JQuery 1 to
> 3
> ​
>
> Thanks
> Victoria & Joao
>
> On Wed, May 23, 2018 at 6:44 AM Aditya Toshniwal <aditya.toshniwal@
> enterprisedb.com> wrote:
>
>> Hi Hackers,
>>
>> Please find the updated patch for upgrading the jQuery version to 3.x.
>> The patch is only for pgAdmin4 code changes replacing old deprecated
>> functions with new replacement.
>> We need to look into external libraries used for any vulnerabilities.
>> Request you to kindly review.
>>
>> Thanks and Regards,
>> Aditya Toshniwal
>> Software Engineer | EnterpriseDB Software Solutions | Pune
>> "Don't Complain about Heat, Plant a tree"
>>
>> On Mon, May 14, 2018 at 1:23 PM, Aditya Toshniwal <aditya.toshniwal@
>> enterprisedb.com> wrote:
>>
>>> Hi Hackers,
>>>
>>> Please hold on with the patch. Will send the updated patch soon.
>>> Apologies.
>>>
>>>
>>>
>>> Thanks and Regards,
>>> Aditya Toshniwal
>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>> "Don't Complain about Heat, Plant a tree"
>>>
>>> On Mon, May 14, 2018 at 12:52 PM, Aditya Toshniwal <aditya.toshniwal@
>>> enterprisedb.com> wrote:
>>>
>>>> Hi Hackers,
>>>>
>>>> PFA patch for upgrading jQuery version to 3.3.1 from current version
>>>> 1.12.4. Patch includes replacing deprecated functions of jquery to latest
>>>> one.
>>>> Kindly review.
>>>>
>>>> Thanks and Regards,
>>>> Aditya Toshniwal
>>>> Software Engineer | EnterpriseDB Software Solutions | Pune
>>>> "Don't Complain about Heat, Plant a tree"
>>>>
>>>
>>>
>>

Reply via email to