Clear step #1386

ataata107 · 2019-12-22T22:26:08Z

Concerns #1369 Clear Step

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
code is in uniquely-named feature branch and has no merge conflicts
PR is descriptively titled
ask @publiclab/is-reviewers for help, in a comment below
Clear-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!

ataata107 · 2019-12-22T22:28:53Z

I have written the code but I am not able to figure out its execution on my pc. I am running npm test-ui-2 but there are no logs.
@jywarren @harshkhandeparkar @keshav234156 please review

harshkhandeparkar · 2019-12-23T17:19:24Z

Does the process end or do you have to press Ctrl + C to kill it?

codecov · 2019-12-23T18:38:41Z

Codecov Report

Merging #1386 into main will decrease coverage by 0.29%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             main    #1386     +/-   ##
=========================================
- Coverage   66.61%   66.31%   -0.3%     
=========================================
  Files         130      125      -5     
  Lines        2672     2574     -98     
  Branches      430      406     -24     
=========================================
- Hits         1780     1707     -73     
+ Misses        892      867     -25

Impacted Files	Coverage Δ
src/modules/ImportImage/Module.js	`12.5% <0%> (-75%)`	⬇️
src/modules/NoiseReduction/NoiseReduction.js	`65.21% <0%> (-34.79%)`	⬇️
src/modules/FlipImage/flipImage.js	`80% <0%> (-20%)`	⬇️
src/util/ParseInputCoordinates.js	`73.33% <0%> (-19.53%)`	⬇️
src/modules/Dither/Dither.js	`58.13% <0%> (-18.61%)`	⬇️
src/util/getStep.js	`10.71% <0%> (-14.29%)`	⬇️
src/modules/FlipImage/Module.js	`89.47% <0%> (-10.53%)`	⬇️
src/modules/ColorTemperature/Module.js	`84.21% <0%> (-10.53%)`	⬇️
src/ui/LoadImage.js	`50% <0%> (-9.1%)`	⬇️
src/modules/AddQR/Module.js	`90.9% <0%> (-9.1%)`	⬇️
... and 43 more

ataata107 · 2019-12-23T22:47:52Z

@harshkhandeparkar
Ok apparantely I was able to get the result of npm run test-ui-2 Mybe it was error of node version as I changed my node version to 8.17.0
As for verifying the clear step I have written the following code

test('Clear Step is pressed', async () => {
   page.on('dialog', async dialog => {
   await dialog.accept();
   });
   const valo = await page.evaluate(() => { document.querySelectorAll('#resetButton')[0].click(); });
   await page.waitForSelector('.step');  
}, timeout);

I am getting the following error

harshkhandeparkar

To fix the catch errors, you will have to do something like

try {
let someVar = await somePromise();
// Rest of the code
}
catch (error) {
console.log(error)
}

ataata107 · 2019-12-24T19:37:31Z

For the following code

const timeout = process.env.SLOWMO ? 30000 : 10000;

const fs = require('fs');
beforeAll(async () => {
  console.log(page.url())
  path = fs.realpathSync('file://../examples/index.html');
  await page.goto('file://' + path, {waitUntil: 'domcontentloaded'});
  // await page.setCacheEnabled(true);
  console.log(page.url())
});

describe('Clear Steps', () => {
  test('Resize Module is added', async () => {
    
    await page.waitForSelector('.step');
    
    const Length = await page.evaluate(() => document.querySelectorAll('.step').length);
    await page.click('[data-value=\'resize\']');
    const Length1 = await page.evaluate(() => document.querySelectorAll('.step').length);
    expect(Length).toBe(1);
    expect(Length1).toBe(2);
  }, timeout);
  test('Brightness Module is added', async () => {
    await page.click('[data-value=\'brightness\']');
    const Length2 = await page.evaluate(() => document.querySelectorAll('.step').length);
    expect(Length2).toBe(3);
  }, timeout);
  test('Contrast Module is added', async () => {
    await page.click('[data-value=\'contrast\']');
    const Length3 = await page.evaluate(() => document.querySelectorAll('.step').length);
    expect(Length3).toBe(4);
  }, timeout);
  test('Saturation Module is added', async () => {
    await page.click('[data-value=\'saturation\']');
    const Length4 = await page.evaluate(() => document.querySelectorAll('.step').length);
    expect(Length4).toBe(5);
  }, timeout);
  test('Rotate Module is added', async () => {
    await page.click('[data-value=\'rotate\']');
    const Length5 = await page.evaluate(() => document.querySelectorAll('.step').length);
    expect(Length5).toBe(6);
  }, timeout);
  test('Crop Module is added', async () => {
    await page.click('[data-value=\'crop\']');
    const Length6 = await page.evaluate(() => document.querySelectorAll('.step').length);
    expect(Length6).toBe(7);
  }, timeout);
  test('Clear Step is pressed', async () => {
    
    
    page.on('dialog', async dialog => {
      console.log(dialog.type());
      (await dialog.accept());
      
    });
    
    // await page.setRequestInterception(true);
    //     page.on('request', interceptedRequest => {
    //         console.log('GOT NEW REQUEST', interceptedRequest.url());
    //         interceptedRequest.continue();
    //     });
    await page.click('#resetButton');
    try {
      // await page.goto('file:///home/ataata107/Desktop/dev/image-sequencer/examples/index.html',{waitUntil: 'domcontentloaded'})
      // let someVar = await page.waitForSelector('.step');
      console.log(page.url())
      // Rest of the code
      
      }
    catch (error) {
      console.log(error)
      }
    
    
  }, timeout);
  
});

The error I get

ataata107@ataata107-Inspiron-5559:~/Desktop/dev/image-sequencer$ node ./node_modules/.bin/jest
  console.log test/ui-2/test/clear_step.test.js:5
    about:blank

 PASS  test/ui-2/test/title.test.js
  Title of the page
    ✓ Title of the page (44ms)

  console.log test/ui-2/test/clear_step.test.js:9
    file:///home/ataata107/Desktop/dev/image-sequencer/examples/index.html

 PASS  test/ui-2/test/radio-group.test.js (14.978s)
  Radio Group
    ✓ Resize Module is added (4352ms)
    ✓ Brightness Module is added (1912ms)
    ✓ Contrast Module is added (2736ms)
    ✓ Saturation Module is added (1383ms)
    ✓ Rotate Module is added (796ms)
    ✓ Crop Module is added (967ms)

 FAIL  test/ui-2/test/clear_step.test.js (15.715s)
  Clear Steps
    ✓ Resize Module is added (5032ms)
    ✓ Brightness Module is added (1086ms)
    ✓ Contrast Module is added (3245ms)
    ✓ Saturation Module is added (1425ms)
    ✓ Rotate Module is added (998ms)
    ✓ Crop Module is added (649ms)
    ✕ Clear Step is pressed (391ms)

  ● Clear Steps › Clear Step is pressed

    ReferenceError: start is not defined

      at file::1:9
      at Suite.Object.<anonymous>.describe (test/ui-2/test/clear_step.test.js:48:3)

  ● Clear Steps › Clear Step is pressed

    ReferenceError: addRow is not defined

      at file::2:9
      at Suite.Object.<anonymous>.describe (test/ui-2/test/clear_step.test.js:48:3)

  ● Clear Steps › Clear Step is pressed

    ReferenceError: addRow is not defined

      at file::3:9
      at Suite.Object.<anonymous>.describe (test/ui-2/test/clear_step.test.js:48:3)

  ● Clear Steps › Clear Step is pressed

    ReferenceError: addRow is not defined

      at file::4:9
      at Suite.Object.<anonymous>.describe (test/ui-2/test/clear_step.test.js:48:3)

Test Suites: 1 failed, 2 passed, 3 total
Tests:       1 failed, 13 passed, 14 total
Snapshots:   0 total
Time:        17.066s
Ran all test suites.
  console.log test/ui-2/test/clear_step.test.js:52
    confirm

  console.log test/ui-2/test/clear_step.test.js:66
    file:///

Shouldn't pressing the clear button redirect to

file:///home/ataata107/Desktop/dev/image-sequencer/examples/index.html

But it redirects to

file:///

harshkhandeparkar · 2019-12-25T09:27:27Z

Ah ok. Look at this code

image-sequencer/examples/demo.js

Lines 85 to 89 in c370a8b

    
           var resetSequence = function () { 
        
             var r = confirm('Do you want to reset the sequence?'); 
        
             if (r) 
        
               window.location = '/'; 
        
           };

It redirects to / which would load the main page if we were using a web(HTTP) URL but we are using file protocol so it redirects to / which is your computer root. I think you should open a separate issue for this.

Or I'll do it. I have a fix in mind.

ataata107 · 2019-12-25T09:46:40Z

Sure

harshkhandeparkar · 2019-12-25T09:47:34Z

#1394

ataata107 · 2019-12-25T12:19:23Z

@harshkhandeparkar
After modifying the changes as recommended in #1394 the clear step test is also working :). I will commit the changes once #1394 is resolved.

harshkhandeparkar · 2019-12-25T12:20:11Z

👍

…

On Wed, 25 Dec, 2019, 5:49 PM Shazeb Ata, ***@***.***> wrote: @harshkhandeparkar <https://github.com/HarshKhandeparkar> After modifying the changes as recommended in #1394 <#1394> the clear step test is also working :). I will commit the changes once #1394 <#1394> is resolved. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1386?email_source=notifications&email_token=AIJI5H2HBB7GPRVWKVXLQP3Q2NFUZA5CNFSM4J6NYWWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHUJ64I#issuecomment-568893297>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIJI5H6WSRSW55PGY3EUMLLQ2NFUZANCNFSM4J6NYWWA> .

ataata107 · 2019-12-29T21:31:01Z

Delete step is added, will uncomment clear step after #1399 is merged

test/ui-2/test/clear_step.test.js

harshkhandeparkar

LGTM! I will approve once the other PR is merged and the clear step test is ready.

jywarren · 2020-01-27T22:20:28Z

Hi, all, how are we doing here? Thank you, this is looking great!!

ataata107 · 2020-01-27T22:22:54Z

Hey, I guess this is ready to be merged.

jywarren · 2020-01-27T22:40:32Z

Thanks! Awesome :-)))) 🎉

ataata107 added 3 commits December 23, 2019 02:22

Added test clear_step

68e7a4e

Merge branch 'main' into clear_step

83e6505

Updated clear step test

1160f80

Changed name of clear step

15fb2ce

Changed name of clear step

b4654c9

harshkhandeparkar requested changes Dec 24, 2019

View reviewed changes

Added delete step

c610a82

harshkhandeparkar requested changes Dec 31, 2019

View reviewed changes

test/ui-2/test/clear_step.test.js Outdated Show resolved Hide resolved

ataata107 and others added 2 commits January 1, 2020 02:29

Added Linebreaks

d81c8a7

Merge branch 'main' into clear_step

cb27c65

harshkhandeparkar reviewed Jan 2, 2020

View reviewed changes

ataata107 added 2 commits January 3, 2020 03:36

Merge branch 'main' into clear_step

fbe4ea3

Added clear and delete step

cfb382f

ataata107 mentioned this pull request Jan 21, 2020

Call for reviewers group and new co-reviewing workflow #656

Open

jywarren approved these changes Jan 27, 2020

View reviewed changes

jywarren added 2 commits January 27, 2020 17:23

Merge branch 'main' into clear_step

d182706

Merge branch 'main' into clear_step

1e82098

jywarren approved these changes Jan 27, 2020

View reviewed changes

jywarren merged commit 79472ba into publiclab:main Jan 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clear step #1386

Clear step #1386

ataata107 commented Dec 22, 2019 •

edited

Loading

ataata107 commented Dec 22, 2019 •

edited

Loading

harshkhandeparkar commented Dec 23, 2019

codecov bot commented Dec 23, 2019 •

edited

Loading

ataata107 commented Dec 23, 2019

harshkhandeparkar left a comment

ataata107 commented Dec 24, 2019

harshkhandeparkar commented Dec 25, 2019

ataata107 commented Dec 25, 2019

harshkhandeparkar commented Dec 25, 2019

ataata107 commented Dec 25, 2019

harshkhandeparkar commented Dec 25, 2019 via email

ataata107 commented Dec 29, 2019

harshkhandeparkar left a comment

jywarren commented Jan 27, 2020

ataata107 commented Jan 27, 2020

jywarren commented Jan 27, 2020

Clear step #1386

Clear step #1386

Conversation

ataata107 commented Dec 22, 2019 • edited Loading

ataata107 commented Dec 22, 2019 • edited Loading

harshkhandeparkar commented Dec 23, 2019

codecov bot commented Dec 23, 2019 • edited Loading

Codecov Report

ataata107 commented Dec 23, 2019

harshkhandeparkar left a comment

Choose a reason for hiding this comment

ataata107 commented Dec 24, 2019

harshkhandeparkar commented Dec 25, 2019

ataata107 commented Dec 25, 2019

harshkhandeparkar commented Dec 25, 2019

ataata107 commented Dec 25, 2019

harshkhandeparkar commented Dec 25, 2019 via email

ataata107 commented Dec 29, 2019

harshkhandeparkar left a comment

Choose a reason for hiding this comment

jywarren commented Jan 27, 2020

ataata107 commented Jan 27, 2020

jywarren commented Jan 27, 2020

ataata107 commented Dec 22, 2019 •

edited

Loading

ataata107 commented Dec 22, 2019 •

edited

Loading

codecov bot commented Dec 23, 2019 •

edited

Loading