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

Basic UI tests -- initial one for quick selector add step #1242

Closed
wants to merge 49 commits into from

Conversation

jywarren
Copy link
Member

@jywarren jywarren commented Sep 1, 2019

re #1000

@jywarren
Copy link
Member Author

jywarren commented Sep 1, 2019

Failures:
1) Default sequencer HTML adds a step from the quick selector
  Message:
    Expected 0 to be 1.
  Stack:
    Error: Expected 0 to be 1.
        at <Jasmine>
        at UserContext.<anonymous> (/home/travis/build/publiclab/image-sequencer/test/ui/spec/defaultHtmlSequencerUi.spec.js:44:31)
        at <Jasmine>
  Message:
    Expected 0 to be 2.
  Stack:
    Error: Expected 0 to be 2.
        at <Jasmine>
        at UserContext.<anonymous> (/home/travis/build/publiclab/image-sequencer/test/ui/spec/defaultHtmlSequencerUi.spec.js:46:31)
        at <Jasmine>

Needs clicking Add Step!

@jywarren
Copy link
Member Author

jywarren commented Sep 1, 2019

Actually no, the quick selector doesn't need an Add Step click. So maybe the lib is not fully set up here yet...

@jywarren
Copy link
Member Author

jywarren commented Sep 1, 2019

We need a fixture to have an HTML page to work on.

@@ -4,6 +4,10 @@ describe('Default sequencer HTML', function() {
var sequencer = require('../../../src/ImageSequencer')();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, here it looks like this is already done in the default UI code. So maybe we need to split this into another file to have one running purely on the fixture file.

@jywarren
Copy link
Member Author

jywarren commented Sep 9, 2019

OK, so, managed to get the fixtures loading. But, I can't seem to get the libraries themselves to load. I'm seeing:

     ReferenceError: Can't find variable: initializeSequencerUi in file:///home/warren/sites/image-sequencer/test/ui/spec/basicUi.spec.js (line 6) (1)

It's odd. I can't get ImageSequencer to load either, despite it's inclusion in the Gruntfile. I also tried using a <script> tag in the fixture, but that didn't work either.

Basing this on https://github.com/publiclab/PublicLab.Editor/blob/master/spec/javascripts/editor_spec.js and https://github.com/publiclab/PublicLab.Editor/blob/master/Gruntfile.js

@publiclab/is-reviewers any suggestions?

@jywarren
Copy link
Member Author

Hi @ananyaarun -- i'm struggling a bit getting these tests to run. I was wondering if you'd be able to look at it since you so recently got this running in LEL? Have I overlooked something simple?

@ananyaarun
Copy link
Member

Hey @jywarren , sure I'll give it a look.

@jywarren
Copy link
Member Author

@Divy123 @harshkhandeparkar @rexagod any ideas here? :-/

@jywarren jywarren mentioned this pull request Sep 23, 2019
3 tasks
@jywarren
Copy link
Member Author

Referencing this similar PR from another project: publiclab/leaflet-environmental-layers#247

@jywarren
Copy link
Member Author

Could it relate to these includes in the file @sagarpreet-chadha set up over in leaflet-environmental-layers? https://github.com/publiclab/leaflet-environmental-layers/pull/206/files#diff-d798dca1a53c3a3773cd503064af3093R15-R39

@vaibhavmatta
Copy link

@jywarren May I help here?

@jywarren
Copy link
Member Author

@vaibhavmatta i'd love help, sorry I didn't see this! I'm just trying to get the most basic browser-based tests running that we can build on!

@jywarren jywarren changed the title UI test for quick selector add step Basic UI tests -- initial one for quick selector add step Nov 18, 2019
@vaibhavmatta
Copy link

vaibhavmatta commented Nov 18, 2019

@jywarren I think basic browser-based tests won't work so efficiently. But still, if you say, I would love to work on them as well.! Thanks 🙂

@jywarren
Copy link
Member Author

jywarren commented Nov 19, 2019 via email

@vaibhavmatta
Copy link

vaibhavmatta commented Nov 19, 2019 via email

@SidharthBansal
Copy link
Member

@keshav234156 is interested in doing this

@jywarren
Copy link
Member Author

jywarren commented Dec 16, 2019 via email

@keshav234156
Copy link
Member

@jywarren @SidharthBansal So I was able to test basic add a step functionality
test file

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

beforeAll(async () => {
    await page.goto("http://192.168.43.171:3000/examples/#steps=", {waitUntil: 'domcontentloaded'});
});

describe('title of the page', () => {
    test('Title of the page', async () => {
        const title = await page.title();
        expect(title).toBe('Image Sequencer');
        
    }, timeout);
});
describe('Add step', () => {
    test('length is increased', async () => {
    	await page.waitForSelector('.step');
        const previousLength = await page.evaluate(() => document.querySelectorAll('.step').length);
        await page.click("[data-value='brightness']")
        const previousLength1 = await page.evaluate(() => document.querySelectorAll('.step').length);
        console.log(previousLength)
        console.log(previousLength1)

        expect(previousLength).toBe(1);
        expect(previousLength1).toBe(2);
   }, timeout);
});

output file

> [email protected] test /home/keshav/image-sequencer
> jest

 PASS  src/test/frontend.test.js
  title of the page
    ✓ Title of the page (10ms)
  Add step
    ✓ length is increased (2269ms)

  console.log src/test/frontend.test.js:20
    1

  console.log src/test/frontend.test.js:21
    2

Test Suites: 1 passed, 1 total
Tests:       2 passed, 2 total
Snapshots:   0 total
Time:        3.804s, estimated 4s
Ran all test suites.

I have done it with the jest puppeteer . I found this as the most suitable here. So we won't require ant HTML fixtures as all the things would be done in headless chrome. What are your view's on this

@jywarren
Copy link
Member Author

jywarren commented Dec 17, 2019 via email

@jywarren
Copy link
Member Author

Solved in #1366 !

@vaibhavmatta
Copy link

vaibhavmatta commented May 9, 2020 via email

@Divy123
Copy link
Member

Divy123 commented May 9, 2020

Seriously, @vaibhavmatta ?? Is this the right platform for this?

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

Successfully merging this pull request may close these issues.

6 participants