Add setInputFiles to ElementHandle #1097

bandorko · 2023-11-10T02:15:56Z

What?

Add setInputFiles support.

Test

Here is a test script you can test this functionality with

index.html: (served by e.g. python3 -m http.server 3080)

<html>

<head>
    <script src="https://unpkg.com/dropzone@5/dist/min/dropzone.min.js"></script>
</head>

<body>
    <!-- Simple file upload form -->
    <form method="POST" action="/upload" enctype="multipart/form-data">
        <input type="file" name="upl" id="simple-upload" multiple />
        <input type="submit" value="Send" />
    </form>

    <!-- Dropzone.js -->
    <div id="dropzone"/>
    <script>
        let myDropzone = new Dropzone("div#dropzone", { url: "/upload", autoProcessQueue:false });
    </script>
</body>

</html>

test.js:

import { browser } from 'k6/experimental/browser';
import { check } from 'k6';

export const options = {
  scenarios: {
    ui: {
      executor: 'shared-iterations',
      vus: 1,
      iterations: 1,
      options: {
        browser: {
          type: 'chromium',
        },
      },
    },
  }
};

function getFileCount(page, id) {
  return page.evaluate((id) => {
    return document.getElementById(id).files.length;
  }, id);
}

function getFileProp(page, id, fileNo, prop) {
  return page.evaluate((id, fileNo, prop) => {
    return document.getElementById(id).files[fileNo][prop];
  }, id, fileNo, prop);
}

function getDropzoneFileCount(page) {
  return page.evaluate(() => {
    return myDropzone.files.length;
  });
}

function getDropzoneFileProp(page, fileNo, prop) {
  return page.evaluate((fileNo, prop) => {
    return myDropzone.files[fileNo][prop];
  }, fileNo, prop);
}

export default async function () {
  const page = browser.newPage();
  try {
    await page.goto('http://localhost:3080/');

    // test simple upload form
    page.setInputFiles('input[id="simple-upload"]', { name: 'test.json', mimetype: 'text/json', buffer: 'MDEyMzQ1Njc4OQ==' });
    check(getFileCount(page, 'simple-upload'), { 'file count is 1': (count) => count === 1 });
    check(getFileProp(page, 'simple-upload', 0, 'name'), { '1st file name is test.json': (prop) => prop === 'test.json' });
    check(getFileProp(page, 'simple-upload', 0, 'type'), { '1st file mimetype is text/json': (prop) => prop === 'text/json' });
    check(getFileProp(page, 'simple-upload', 0, 'size'), { '1st file size is 10': (prop) => prop === 10 });

    page.setInputFiles('input[id="simple-upload"]', [{ name: 'test.json', mimetype: 'text/json', buffer: 'MDEyMzQ1Njc4OQ==' }, { name: 'hello.txt', mimetype: 'text/plain', buffer: 'SGVsbG8gSzYgV29ybGQK' }]);
    check(getFileCount(page, 'simple-upload'), { 'file count is 2': (count) => count === 2 });
    check(getFileProp(page, 'simple-upload', 0, 'name'), { '1st file name is test.json': (prop) => prop === 'test.json' });
    check(getFileProp(page, 'simple-upload', 0, 'type'), { '1st file mimetype is text/json': (prop) => prop === 'text/json' });
    check(getFileProp(page, 'simple-upload', 0, 'size'), { '1st file size is 10': (prop) => prop === 10 });
    check(getFileProp(page, 'simple-upload', 1, 'name'), { '2nd file name is hello.txt': (prop) => prop === 'hello.txt' });
    check(getFileProp(page, 'simple-upload', 1, 'type'), { '2nd file mimetype is text/plain': (prop) => prop === 'text/plain' });
    check(getFileProp(page, 'simple-upload', 1, 'size'), { '2nd file size is 15': (prop) => prop === 15 });

    page.setInputFiles('input[id="simple-upload"]', null);
    check(getFileCount(page, 'simple-upload'), { 'file count is 0': (count) => count === 0 });

    page.setInputFiles('input[id="simple-upload"]', 'test.js');
    check(getFileCount(page, 'simple-upload'), { 'file count is 1': (count) => count === 1 });
    check(getFileProp(page, 'simple-upload', 0, 'name'), { '1st file name is test.js': (prop) => prop === 'test.js' });
    check(getFileProp(page, 'simple-upload', 0, 'type'), { '1st file mimetype is text/javascript; charset=utf-8': (prop) => prop === 'text/javascript; charset=utf-8' });
    check(getFileProp(page, 'simple-upload', 0, 'size'), { '1st file size is larger than 1000': (prop) => prop > 1000 });

    page.setInputFiles('input[id="simple-upload"]', ['test.js', 'test.js']);
    check(getFileCount(page, 'simple-upload'), { 'file count is 2': (count) => count === 2 });

    // test dropzone.js form
    page.setInputFiles('input[class="dz-hidden-input"]', { name: 'test.json', mimetype: 'text/json', buffer: 'MDEyMzQ1Njc4OQ==' });
    check(getDropzoneFileCount(page), { 'dropzone file count is 1': (count) => count === 1 });
    check(getDropzoneFileProp(page, 0, 'name'), { 'dropzone 1st file name is test.json': (prop) => prop === 'test.json' });
    check(getDropzoneFileProp(page, 0, 'type'), { 'dropzone 1st file mimetype is text/json': (prop) => prop === 'text/json' });
    check(getDropzoneFileProp(page, 0, 'size'), { 'dropzone 1st file size is 10': (prop) => prop === 10 });
  } finally {
    page.close();
  }
}

Why?

Users can select input files for upload.

Checklist

I have performed a self-review of my code
I have added tests for my changes
I have commented on my code, particularly in hard-to-understand areas

Related PR(s)/Issue(s)

Closes: #384

bandorko · 2023-11-10T02:24:18Z

I know that this is far from ready, just please give me feedback, if I am on the right track?

It works like this:

import { browser } from 'k6/experimental/browser';
import { sleep } from 'k6';
import encoding from 'k6/encoding';

export const options = {
  scenarios: {
    ui: {
      executor: 'shared-iterations',
      vus: 1,
      iterations: 1,
      options: {
        browser: {
          type: 'chromium',
        },
      },
    },
  }
}

export default async function () {
  const page = browser.newPage();
  try {
    await page.goto('http://localhost:9080');
    const input = page.$('input[name="upload"]');
    input.setInputFiles({ payload: [{ name: 'filename.txt', buffer: encoding.b64encode('this is test'), mime: 'text/plain' },{ name: 'filename2.txt', buffer: encoding.b64encode('this is second test'), mime: 'text/plain' }] });
    sleep(20);
  } finally {
    page.close();
  }
}

with this html:

<html>
    <input type="file" name="upload"/>
</html>

inancgumus

Thanks, @bandorko. It looks LGTM so far 👏 Added some suggestions. We also need more tests to ensure that this works. Please also use our commit message conventions and add descriptions to your commits. You might also want to consider this issue next :)

common/element_handle_options.go

bandorko · 2023-12-03T23:34:01Z

@inancgumus
I implemented SetInputFiles in Frame and Page also and I created tests for the functionality.

inancgumus

Mostly LGTM 👏 Thanks for the changes. I've added some suggestions.

common/element_handle_options.go

common/element_handle.go

common/element_handle_options.go

common/frame.go

tests/page_test.go

ankur22

Thank you for the contribution! 🎉 This should be quite useful for everyone when they wish to test uploading of files from their websites 🙇

I've left some comments for you to ponder over and work on if you think they are valid.

common/element_handle.go

common/js/injected_script.js

common/element_handle.go

common/js/injected_script.js

common/frame.go

ankur22

Awesome work 👏 thanks for the contribution!

I'm wondering if you could add the details of a test script that you've used to test this change in the PR description for posterity reasons?

I've also left a nit-pick which i'm very happy for you to ignore.

common/element_handle_options.go

ka3de

Great job @bandorko ! Thank you for your contribution.
LGTM after the existing conflicts are resolved.

inancgumus

Great work and initiative. Thanks for your contribution 🙇

bandorko · 2024-01-19T12:48:46Z

@ankur22 , @ka3de , @inancgumus : Thank you!
I still want to fix the issues @ankur22 found, and there are still lint issues I don't know how to handle:

common/element_handle_options.go:214:2  exhaustive  missing cases in switch of type reflect.Kind: reflect.Invalid, reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr, reflect.Float32, reflect.Float64, reflect.Complex64, reflect.Complex128, reflect.Array, reflect.Chan, reflect.Func, reflect.Interface, reflect.Pointer|reflect.Ptr, reflect.Slice, reflect.Struct, reflect.UnsafePointer
common/element_handle_options.go:240:2  exhaustive  missing cases in switch of type reflect.Kind: reflect.Invalid, reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr, reflect.Float32, reflect.Float64, reflect.Complex64, reflect.Complex128, reflect.Array, reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer|reflect.Ptr, reflect.String, reflect.Struct, reflect.UnsafePointer

inancgumus · 2024-01-30T05:08:11Z

Hi @bandorko,

there are still lint issues I don't know how to handle

What difficulties are you having with the linter issues? We can help. It would be great if we could wrap this PR up for the next version 👍

ankur22 · 2024-01-31T08:59:49Z

@bandorko I think it might be ok to add //nolint: exhaustive for this particular case. WDYT @inancgumus?

bandorko · 2024-01-31T21:26:55Z

@bandorko I think it might be ok to add //nolint: exhaustive for this particular case. WDYT @inancgumus?

There is an other option you may want to consider: setting default-signifies-exhaustive: true for exhaustive linter

inancgumus · 2024-02-01T07:57:14Z

There is an other option you may want to consider: setting default-signifies-exhaustive: true for exhaustive linter

Thanks, @bandorko, for the suggestion. But I'm on the side of keeping the exhaustive linter turned on (w/o a default turn off-er) as it allows us to catch bugs early. WDYT @ankur22?

ankur22 · 2024-02-01T09:49:30Z

Thanks, @bandorko, for the suggestion. But I'm on the side of keeping the exhaustive linter turned on (w/o a default turn off-er) as it allows us to catch bugs early. WDYT @ankur22?

Agreed 👍 Let's keep it the same for now and review each on a case-by-case basis.

bandorko · 2024-02-02T20:50:32Z

@inancgumus , @ankur22 I fixed the exhaustive lint issue as you suggested. What else can I do for this PR?

inancgumus · 2024-02-05T07:56:13Z

Hi @bandorko, we can merge this PR after fixing the conflicts with the main branch. Please let us know when you're done. Thanks!

bandorko · 2024-02-17T08:57:46Z

@inancgumus
I added a test script to the description as @ankur22 asked a month ago, so I think I am done with this PR.

inancgumus · 2024-02-19T09:06:51Z

Hi @bandorko,

I added a test script to the description as @ankur22 asked a month ago, so I think I am done with this PR.

That's great. Would you like to resolve the conflicts? Or we can take it over from here? WDYT?

This branch has conflicts that must be resolved
Use the web editor or the to resolve conflicts.
Conflicting files
tests/page_test.go

bandorko · 2024-02-20T08:40:24Z

As I see, it is not just a simple merge conflict, because there were refactors recently. So I will try to resolve the issues.

inancgumus · 2024-03-06T14:21:22Z

I can't merge this PR due to this message:

"This branch cannot be rebased due to conflicts"

@bandorko, can I work on your personal fork (bandorko:feature/384-setInputFiles) and push there first to fix the issues on your behalf?

bandorko · 2024-03-06T15:19:35Z

@inancgumus Yes, of course. Should I give you permission for my fork?

inancgumus · 2024-03-07T09:05:13Z

@inancgumus Yes, of course. Should I give you permission for my fork?

@bandorko Yes, please, thanks!

bandorko · 2024-03-07T10:32:32Z

@inancgumus I already gave permission yesterday

inancgumus · 2024-03-07T11:21:42Z

@bandorko I couldn't resolve the conflicts. There are conflicts to resolve when we do:

# in your fork
git checkout main
git pull upstream main
gc feature/384-setInputFiles
git rebase main

ankur22 · 2024-03-07T11:52:01Z

@bandorko I hope it's ok if we take over this and do some reorganising of the commits, including dropping some commits and other cleanups.

support playwright parameters for setInputFiles(): string|Array<string>|Object|Array<Object>

Co-authored-by: Ankur <ankur.agarwal@grafana.com>

The base branch was changed.

erashdan · 2024-03-11T09:40:16Z

works for me thanks ! 👍🏻

inancgumus reviewed Nov 29, 2023

View reviewed changes

common/element_handle_options.go Outdated Show resolved Hide resolved

common/element_handle_options.go Outdated Show resolved Hide resolved

common/element_handle_options.go Outdated Show resolved Hide resolved

inancgumus changed the title ~~add setInputFiles to ElementHandle~~ Add setInputFiles to ElementHandle Nov 29, 2023

bandorko force-pushed the feature/384-setInputFiles branch from e9ba8ba to a60419b Compare December 1, 2023 23:30

bandorko marked this pull request as draft December 1, 2023 23:58

bandorko marked this pull request as ready for review December 3, 2023 23:27

inancgumus reviewed Dec 21, 2023

View reviewed changes

inancgumus requested review from ka3de and ankur22 January 8, 2024 08:56

ankur22 requested changes Jan 11, 2024

View reviewed changes

bandorko force-pushed the feature/384-setInputFiles branch from d5ce745 to bc2a0c9 Compare January 12, 2024 22:36

ankur22 previously approved these changes Jan 16, 2024

View reviewed changes

common/element_handle_options.go Show resolved Hide resolved

ka3de previously approved these changes Jan 18, 2024

View reviewed changes

inancgumus previously approved these changes Jan 19, 2024

View reviewed changes

inancgumus assigned bandorko Jan 30, 2024

bandorko dismissed stale reviews from inancgumus, ka3de, and ankur22 via 6e49143 February 2, 2024 20:48

inancgumus reopened this Mar 6, 2024

bandorko added 4 commits March 7, 2024 15:58

Add setInputFiles to ElementHandle

fd34f95

Add capability to set input files in multiple form

95018a9

support playwright parameters for setInputFiles(): string|Array<string>|Object|Array<Object>

Refactor: separate SetInputFiles parameter from options

42a56bd

Add SetInputFiles functionality to Page and Frame

97ac027

inancgumus force-pushed the feature/384-setInputFiles branch from bf4a59d to c479205 Compare March 7, 2024 12:59

bandorko and others added 4 commits March 7, 2024 16:05

Add test for SetInputFiles function

bd77af5

Add tests for SetInputFiles functionality in ElementHandle

7d4319e

Add tests for setInputFiles() in the injected script

3d1d6f4

Fix ElementHandle.setInputFiles

61bd668

Co-authored-by: Ankur <ankur.agarwal@grafana.com>

inancgumus force-pushed the feature/384-setInputFiles branch from c479205 to 61bd668 Compare March 7, 2024 13:05

inancgumus changed the base branch from feature/384-setInputFiles to main March 7, 2024 13:06

inancgumus requested review from ankur22 and inancgumus March 7, 2024 13:08

inancgumus approved these changes Mar 7, 2024

View reviewed changes

ankur22 approved these changes Mar 7, 2024

View reviewed changes

inancgumus merged commit a055418 into grafana:main Mar 7, 2024
14 checks passed

ankur22 mentioned this pull request Mar 11, 2024

Add SetInputFiles support #384

Closed

inancgumus deleted the feature/384-setInputFiles branch March 11, 2024 14:34

ankur22 mentioned this pull request Mar 20, 2024

Add setInputFiles docs for browser module grafana/k6-docs#1528

Merged

5 tasks

BrewTestBot mentioned this pull request Mar 25, 2024

k6 0.50.0 Homebrew/homebrew-core#167104

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add setInputFiles to ElementHandle #1097

Add setInputFiles to ElementHandle #1097

bandorko commented Nov 10, 2023 •

edited

bandorko commented Nov 10, 2023

inancgumus left a comment •

edited

bandorko commented Dec 3, 2023

inancgumus left a comment •

edited

ankur22 left a comment

ankur22 left a comment

ka3de left a comment

inancgumus left a comment

bandorko commented Jan 19, 2024

inancgumus commented Jan 30, 2024 •

edited

ankur22 commented Jan 31, 2024

bandorko commented Jan 31, 2024

inancgumus commented Feb 1, 2024 •

edited

ankur22 commented Feb 1, 2024

bandorko commented Feb 2, 2024

inancgumus commented Feb 5, 2024

bandorko commented Feb 17, 2024

inancgumus commented Feb 19, 2024

bandorko commented Feb 20, 2024

inancgumus commented Mar 6, 2024 •

edited

bandorko commented Mar 6, 2024

inancgumus commented Mar 7, 2024

bandorko commented Mar 7, 2024

inancgumus commented Mar 7, 2024

ankur22 commented Mar 7, 2024

erashdan commented Mar 11, 2024 •

edited

Add setInputFiles to ElementHandle #1097

Add setInputFiles to ElementHandle #1097

Conversation

bandorko commented Nov 10, 2023 • edited

What?

Test

Why?

Checklist

Related PR(s)/Issue(s)

bandorko commented Nov 10, 2023

inancgumus left a comment • edited

Choose a reason for hiding this comment

bandorko commented Dec 3, 2023

inancgumus left a comment • edited

Choose a reason for hiding this comment

ankur22 left a comment

Choose a reason for hiding this comment

ankur22 left a comment

Choose a reason for hiding this comment

ka3de left a comment

Choose a reason for hiding this comment

inancgumus left a comment

Choose a reason for hiding this comment

bandorko commented Jan 19, 2024

inancgumus commented Jan 30, 2024 • edited

ankur22 commented Jan 31, 2024

bandorko commented Jan 31, 2024

inancgumus commented Feb 1, 2024 • edited

ankur22 commented Feb 1, 2024

bandorko commented Feb 2, 2024

inancgumus commented Feb 5, 2024

bandorko commented Feb 17, 2024

inancgumus commented Feb 19, 2024

bandorko commented Feb 20, 2024

inancgumus commented Mar 6, 2024 • edited

bandorko commented Mar 6, 2024

inancgumus commented Mar 7, 2024

bandorko commented Mar 7, 2024

inancgumus commented Mar 7, 2024

ankur22 commented Mar 7, 2024

erashdan commented Mar 11, 2024 • edited

bandorko commented Nov 10, 2023 •

edited

inancgumus left a comment •

edited

inancgumus left a comment •

edited

inancgumus commented Jan 30, 2024 •

edited

inancgumus commented Feb 1, 2024 •

edited

inancgumus commented Mar 6, 2024 •

edited

erashdan commented Mar 11, 2024 •

edited