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

[fs] Add file.read and file.seek methods to the fs module (2/3) #3309

Merged
merged 12 commits into from
Nov 6, 2023

Conversation

oleiade
Copy link
Member

@oleiade oleiade commented Aug 29, 2023

What?

This Pull Request builds (and is based) upon #3142 and adds a read and a seek method, the the File object exposed by the k6/experimental/fs module.

read

The File.read method allows to read a file into a Uint8Array. The amount of bytes read is equal to the size of the provided buffer.

The method resolves to either the number of bytes read during the operation or EOF (represented as null) if there is nothing more to read.

seek

The File.seek method allows to seek to a specified offset within a file under the given whence.

The offset is expressed in bytes. Whence allows you to seek from the start of the file (the offset needs to be strictly negative), the current position within the file (offset can be either positive or negative), or from the end of the file (offset needs to be strictly negative). A dedicated SeekMode type/enum is exposed to help with it.

The method resolves to the new offset in bytes after the seek operation.

In action

import { open } from "k6/experimental/fs";

export const options = {
	vus: 100,
	iterations: 1000,
};

// As k6 does not support asynchronous code in the init context, yet, we need to
// use a top-level async function to be able to use the `await` keyword.
let file;
(async function () {
	file = await open("bonjour.txt");
})();

export default async function () {
	// About information about the file
	const fileinfo = await file.stat();
	if (fileinfo.name != "bonjour.txt") {
		throw new Error("Unexpected file name");
	}

	// Define a buffer of the same size as the file
	// to read the file content into.
	const buffer = new Uint8Array(fileinfo.size);

	// Read the file's content into the buffer
	const bytesRead = await file.read(buffer);

	// Check that we read the expected number of bytes
	if (bytesRead != fileinfo.size) {
		throw new Error("Unexpected number of bytes read");
	}

	// Seek back to the beginning of the file
	await file.seek(0);
}

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make ci-like-lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

ref #3142
ref #2974
fixes #3145
fixes #3148

@oleiade oleiade self-assigned this Aug 29, 2023
@oleiade oleiade added this to the v0.47.0 milestone Aug 29, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (experimental/fs@79ba8f2). Click here to learn what that means.

❗ Current head f04fce1 differs from pull request most recent head 922bbfa. Consider uploading reports for the commit 922bbfa to get more accurate results

Additional details and impacted files
@@                Coverage Diff                 @@
##             experimental/fs    #3309   +/-   ##
==================================================
  Coverage                   ?   73.11%           
==================================================
  Files                      ?      261           
  Lines                      ?    20092           
  Branches                   ?        0           
==================================================
  Hits                       ?    14690           
  Misses                     ?     4460           
  Partials                   ?      942           
Flag Coverage Δ
ubuntu 73.11% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@oleiade oleiade changed the title [2/2] Add a read method to the underlying file type [2/2] Add file.read and file.seek methods to the fs module Aug 30, 2023
@oleiade oleiade changed the title [2/2] Add file.read and file.seek methods to the fs module Add file.read and file.seek methods to the fs module Aug 30, 2023
@oleiade oleiade changed the title Add file.read and file.seek methods to the fs module [fs] Add file.read and file.seek methods to the fs module Aug 30, 2023
examples/experimental/fs/fs.js Outdated Show resolved Hide resolved
examples/experimental/fs/fs.js Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/module_test.go Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/module.go Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/module.go Outdated Show resolved Hide resolved
@olegbespalov
Copy link
Contributor

I did a round of reviewing, mostly looks good 👍 waiting for Mihail's comment to be addressed.

@oleiade oleiade changed the title [fs] Add file.read and file.seek methods to the fs module [fs] Add file.read and file.seek methods to the fs module (2/3) Oct 10, 2023
@olegbespalov
Copy link
Contributor

Just for the record.

I'm OK with merging this and working on the fix in a separate PR. The tiny commit that could be done is to write the test case, which should catch the bug and t.Skip it for now.

My rationale is that merging this will put the focus on fixing the bug. And we still have plenty of time until the release; in the worst case, we could be transparent that a known issue exists. However, some of the users of this experiment already could try to use it and provide feedback.

js/modules/k6/experimental/fs/module.go Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/module.go Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/module.go Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/module.go Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/module.go Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/module.go Outdated Show resolved Hide resolved
js/modules/k6/experimental/fs/file.go Outdated Show resolved Hide resolved
@oleiade oleiade requested a review from mstoykov November 6, 2023 13:49
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

LGTM!

js/modules/k6/experimental/fs/module.go Show resolved Hide resolved
@oleiade oleiade merged commit ad96bd8 into experimental/fs Nov 6, 2023
22 checks passed
@oleiade oleiade deleted the experimental/fs-readseek branch November 6, 2023 16:26
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.

Implement the File API's File.seek method Implement the File API's File.read method
5 participants