Skip to content

Added writeBig* methods#100

Merged
dop251 merged 3 commits intodop251:masterfrom
stampy88:write_big_int_uint
Mar 6, 2025
Merged

Added writeBig* methods#100
dop251 merged 3 commits intodop251:masterfrom
stampy88:write_big_int_uint

Conversation

@stampy88
Copy link
Copy Markdown
Contributor

@stampy88 stampy88 commented Mar 3, 2025

  • added writeBigInt64BE, writeBigInt64LE, writeBigUInt64BE, and writeBigUInt64LE along with their aliases

- added `writeBigInt64BE`, `writeBigInt64LE`, `writeBigUInt64BE`, and `writeBigUInt64LE` along with their aliases
@stampy88
Copy link
Copy Markdown
Contributor Author

stampy88 commented Mar 4, 2025

@dop251 wanted your thoughts on something. These tests are getting larger and larger, so I had the inclination to define some assertion utility functions in JavaScript that I could use instead of repeating similar JS. An example would be to replace something like:

name: "write with default offset",
script: `
	const buf = Buffer.alloc(16);
	const bytesWritten = buf.writeBigInt64BE(BigInt(123456789));
	
	if (bytesWritten !== 8) {
		throw new Error('bytesWritten should be 8');
	}
	
	const value = buf.readBigInt64BE(0);
	if (value !== BigInt(123456789)) {
		throw new Error('should read back the written value, got: ' + value);
	}
`,

With

name: "write with default offset",
script: `
    const buf = Buffer.alloc(16);
    assertBufferWriteRead(buf, 'writeBigInt64BE', 'readBigInt64BE', BigInt(123456789)); 
`,

My concern was that it makes it less obvious exactly what you are testing.

@stampy88
Copy link
Copy Markdown
Contributor Author

stampy88 commented Mar 4, 2025

other option would be to define finer grained assertion functions, e.g. assertBytesWritten(bytesWritten, 8) -> throws that error

@dop251
Copy link
Copy Markdown
Owner

dop251 commented Mar 4, 2025

Makes sense to me. In general I think most of the assertions need to be done in js, rather than in Go, there is even assert.js with some useful methods. Feel free to add there if you think it could be used by other packages too.

@stampy88
Copy link
Copy Markdown
Contributor Author

stampy88 commented Mar 4, 2025

Ah, I didn't even notice that assert.js file. Thanks, I'll see what I can do.

@stampy88 stampy88 force-pushed the write_big_int_uint branch from 90090d2 to 958509e Compare March 5, 2025 19:35
- added the `assertions.js` file for the buffer tests
- refactored the `assert.throws*` method to reduce the duplicate code
@stampy88 stampy88 force-pushed the write_big_int_uint branch from 958509e to 243134a Compare March 5, 2025 19:38
@stampy88
Copy link
Copy Markdown
Contributor Author

stampy88 commented Mar 5, 2025

@dop251 this is good to go. I'll probably do some more test refactoring on the next PR

@dop251 dop251 merged commit a019936 into dop251:master Mar 6, 2025
@stampy88 stampy88 deleted the write_big_int_uint branch March 6, 2025 16:44
benallfree pushed a commit to benallfree/goja_nodejs that referenced this pull request May 3, 2025
* Added writeBig* methods
- added `writeBigInt64BE`, `writeBigInt64LE`, `writeBigUInt64BE`, and `writeBigUInt64LE` along with their aliases

* Simplified tests
- added the `assertions.js` file for the buffer tests
- refactored the `assert.throws*` method to reduce the duplicate code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants