-
-
Notifications
You must be signed in to change notification settings - Fork 396
Add initial specs for IO::Buffer #1297
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
Changes from all commits
bd098d7
0500027
f59b2a6
eaecca6
848f44d
9e6959a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| require_relative '../../../spec_helper' | ||
| require_relative 'shared/null_and_empty' | ||
|
|
||
| describe "IO::Buffer#empty?" do | ||
| after :each do | ||
| @buffer&.free | ||
| @buffer = nil | ||
| end | ||
|
|
||
| it_behaves_like :io_buffer_null_and_empty, :empty? | ||
|
|
||
| it "is true for a 0-length String-backed buffer created with .for" do | ||
| @buffer = IO::Buffer.for("") | ||
| @buffer.empty?.should be_true | ||
| end | ||
|
|
||
| ruby_version_is "3.3" do | ||
| it "is true for a 0-length String-backed buffer created with .string" do | ||
| IO::Buffer.string(0) do |buffer| | ||
| buffer.empty?.should be_true | ||
| end | ||
| end | ||
| end | ||
|
|
||
| it "is true for a 0-length slice of a buffer with size > 0" do | ||
| @buffer = IO::Buffer.new(4) | ||
| @buffer.slice(3, 0).empty?.should be_true | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| require_relative '../../../spec_helper' | ||
|
|
||
| describe "IO::Buffer#external?" do | ||
| after :each do | ||
| @buffer&.free | ||
| @buffer = nil | ||
| end | ||
|
|
||
| context "with a buffer created with .new" do | ||
| it "is false for an internal buffer" do | ||
| @buffer = IO::Buffer.new(4) | ||
| @buffer.external?.should be_false | ||
| end | ||
|
|
||
| it "is false for a mapped buffer" do | ||
| @buffer = IO::Buffer.new(4, IO::Buffer::MAPPED) | ||
| @buffer.external?.should be_false | ||
| end | ||
| end | ||
|
|
||
| context "with a file-backed buffer created with .map" do | ||
| it "is true for a regular mapping" do | ||
| File.open(__FILE__, "r") do |file| | ||
| @buffer = IO::Buffer.map(file, nil, 0, IO::Buffer::READONLY) | ||
| @buffer.external?.should be_true | ||
| end | ||
| end | ||
|
|
||
| ruby_version_is "3.3" do | ||
| it "is false for a private mapping" do | ||
| File.open(__FILE__, "r") do |file| | ||
| @buffer = IO::Buffer.map(file, nil, 0, IO::Buffer::READONLY | IO::Buffer::PRIVATE) | ||
| @buffer.external?.should be_false | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context "with a String-backed buffer created with .for" do | ||
| it "is true for a buffer created without a block" do | ||
| @buffer = IO::Buffer.for("test") | ||
| @buffer.external?.should be_true | ||
| end | ||
|
|
||
| it "is true for a buffer created with a block" do | ||
| IO::Buffer.for(+"test") do |buffer| | ||
| buffer.external?.should be_true | ||
| end | ||
| end | ||
| end | ||
|
|
||
| ruby_version_is "3.3" do | ||
| context "with a String-backed buffer created with .string" do | ||
| it "is true" do | ||
| IO::Buffer.string(4) do |buffer| | ||
| buffer.external?.should be_true | ||
| end | ||
| end | ||
| end | ||
| end | ||
|
|
||
| # Always false for slices | ||
| context "with a slice of a buffer" do | ||
| context "created with .new" do | ||
| it "is false when slicing an internal buffer" do | ||
| @buffer = IO::Buffer.new(4) | ||
| @buffer.slice.external?.should be_false | ||
| end | ||
|
|
||
| it "is false when slicing a mapped buffer" do | ||
| @buffer = IO::Buffer.new(4, IO::Buffer::MAPPED) | ||
| @buffer.slice.external?.should be_false | ||
| end | ||
| end | ||
|
|
||
| context "created with .map" do | ||
| it "is false" do | ||
| File.open(__FILE__, "r") do |file| | ||
| @buffer = IO::Buffer.map(file, nil, 0, IO::Buffer::READONLY) | ||
| @buffer.slice.external?.should be_false | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context "created with .for" do | ||
| it "is false when slicing a buffer created without a block" do | ||
| @buffer = IO::Buffer.for("test") | ||
| @buffer.slice.external?.should be_false | ||
| end | ||
|
|
||
| it "is false when slicing a buffer created with a block" do | ||
| IO::Buffer.for(+"test") do |buffer| | ||
| buffer.slice.external?.should be_false | ||
| end | ||
| end | ||
| end | ||
|
|
||
| ruby_version_is "3.3" do | ||
| context "created with .string" do | ||
| it "is false" do | ||
| IO::Buffer.string(4) do |buffer| | ||
| buffer.slice.external?.should be_false | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| require_relative '../../../spec_helper' | ||
|
|
||
| describe "IO::Buffer#free" do | ||
| context "with a buffer created with .new" do | ||
| it "frees internal memory and nullifies the buffer" do | ||
| buffer = IO::Buffer.new(4) | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| end | ||
|
|
||
| it "frees mapped memory and nullifies the buffer" do | ||
| buffer = IO::Buffer.new(4, IO::Buffer::MAPPED) | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| end | ||
| end | ||
|
|
||
| context "with a file-backed buffer created with .map" do | ||
| it "frees mapped memory and nullifies the buffer" do | ||
| File.open(__FILE__, "r") do |file| | ||
| buffer = IO::Buffer.map(file, nil, 0, IO::Buffer::READONLY) | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context "with a String-backed buffer created with .for" do | ||
| context "without a block" do | ||
| it "disassociates the buffer from the string and nullifies the buffer" do | ||
| string = +"test" | ||
| buffer = IO::Buffer.for(string) | ||
| # Read-only buffer, can't modify the string. | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| end | ||
| end | ||
|
|
||
| context "with a block" do | ||
| it "disassociates the buffer from the string and nullifies the buffer" do | ||
| string = +"test" | ||
| IO::Buffer.for(string) do |buffer| | ||
| buffer.set_string("meat") | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| end | ||
| string.should == "meat" | ||
| end | ||
| end | ||
| end | ||
|
|
||
| ruby_version_is "3.3" do | ||
| context "with a String-backed buffer created with .string" do | ||
| it "disassociates the buffer from the string and nullifies the buffer" do | ||
| string = | ||
| IO::Buffer.string(4) do |buffer| | ||
| buffer.set_string("meat") | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| end | ||
| string.should == "meat" | ||
| end | ||
| end | ||
| end | ||
|
|
||
| it "can be called repeatedly without an error" do | ||
| buffer = IO::Buffer.new(4) | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| end | ||
|
|
||
| it "is disallowed while locked, raising IO::Buffer::LockedError" do | ||
| buffer = IO::Buffer.new(4) | ||
| buffer.locked do | ||
| -> { buffer.free }.should raise_error(IO::Buffer::LockedError, "Buffer is locked!") | ||
| end | ||
| buffer.free | ||
| buffer.null?.should be_true | ||
| end | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor: seems it's duplicated in specs for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and other dupes: I thought it would be better to over-spec than to select which file these interactions shoud be in 😅 I guess it would make sense to remove them from
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, excessive tests shouldn't be a problem. Yes, I agree that this specs would be better to put only here. The main criterion as for me is to look from the user point of view - where a Ruby developer would look for these specs if he wants to checks the logic. |
||
|
|
||
| context "with a slice of a buffer" do | ||
| it "nullifies the slice, not touching the buffer" do | ||
| buffer = IO::Buffer.new(4) | ||
| slice = buffer.slice(0, 2) | ||
|
|
||
| slice.free | ||
| slice.null?.should be_true | ||
| buffer.null?.should be_false | ||
|
|
||
| buffer.free | ||
| end | ||
|
|
||
| it "nullifies buffer, invalidating the slice" do | ||
| buffer = IO::Buffer.new(4) | ||
| slice = buffer.slice(0, 2) | ||
|
|
||
| buffer.free | ||
| slice.null?.should be_false | ||
| slice.valid?.should be_false | ||
| end | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor: seems is duplicated in specs for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't really remove duplication, but cut off excess lines in this one and the one above. |
||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| require_relative '../../../spec_helper' | ||
|
|
||
| describe "IO::Buffer#initialize" do | ||
| after :each do | ||
| @buffer&.free | ||
| @buffer = nil | ||
| end | ||
|
|
||
| it "creates a new zero-filled buffer with default size" do | ||
| @buffer = IO::Buffer.new | ||
| @buffer.size.should == IO::Buffer::DEFAULT_SIZE | ||
| @buffer.each(:U8).should.all? { |_offset, value| value.eql?(0) } | ||
| end | ||
|
|
||
| it "creates a buffer with default state" do | ||
| @buffer = IO::Buffer.new | ||
| @buffer.should_not.shared? | ||
| @buffer.should_not.readonly? | ||
|
|
||
| @buffer.should_not.empty? | ||
| @buffer.should_not.null? | ||
|
|
||
| # This is run-time state, set by #locked. | ||
| @buffer.should_not.locked? | ||
| end | ||
|
|
||
| context "with size argument" do | ||
| it "creates a new internal buffer if size is less than IO::Buffer::PAGE_SIZE" do | ||
| size = IO::Buffer::PAGE_SIZE - 1 | ||
| @buffer = IO::Buffer.new(size) | ||
| @buffer.size.should == size | ||
| @buffer.should.internal? | ||
| @buffer.should_not.mapped? | ||
| @buffer.should_not.empty? | ||
| end | ||
|
|
||
| it "creates a new mapped buffer if size is greater than or equal to IO::Buffer::PAGE_SIZE" do | ||
| size = IO::Buffer::PAGE_SIZE | ||
| @buffer = IO::Buffer.new(size) | ||
| @buffer.size.should == size | ||
| @buffer.should_not.internal? | ||
| @buffer.should.mapped? | ||
| @buffer.should_not.empty? | ||
| end | ||
andrykonchin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| it "creates a null buffer if size is 0" do | ||
| @buffer = IO::Buffer.new(0) | ||
| @buffer.size.should.zero? | ||
| @buffer.should_not.internal? | ||
| @buffer.should_not.mapped? | ||
| @buffer.should.null? | ||
| @buffer.should.empty? | ||
| end | ||
|
|
||
| it "raises TypeError if size is not an Integer" do | ||
| -> { IO::Buffer.new(nil) }.should raise_error(TypeError, "not an Integer") | ||
| -> { IO::Buffer.new(10.0) }.should raise_error(TypeError, "not an Integer") | ||
| end | ||
|
|
||
| it "raises ArgumentError if size is negative" do | ||
| -> { IO::Buffer.new(-1) }.should raise_error(ArgumentError, "Size can't be negative!") | ||
| end | ||
| end | ||
|
|
||
| context "with size and flags arguments" do | ||
| it "forces mapped buffer with IO::Buffer::MAPPED flag" do | ||
| @buffer = IO::Buffer.new(IO::Buffer::PAGE_SIZE - 1, IO::Buffer::MAPPED) | ||
| @buffer.should.mapped? | ||
| @buffer.should_not.internal? | ||
| @buffer.should_not.empty? | ||
| end | ||
|
|
||
| it "forces internal buffer with IO::Buffer::INTERNAL flag" do | ||
| @buffer = IO::Buffer.new(IO::Buffer::PAGE_SIZE, IO::Buffer::INTERNAL) | ||
| @buffer.should.internal? | ||
| @buffer.should_not.mapped? | ||
| @buffer.should_not.empty? | ||
| end | ||
|
|
||
| it "raises IO::Buffer::AllocationError if neither IO::Buffer::MAPPED nor IO::Buffer::INTERNAL is given" do | ||
| -> { IO::Buffer.new(10, IO::Buffer::READONLY) }.should raise_error(IO::Buffer::AllocationError, "Could not allocate buffer!") | ||
| -> { IO::Buffer.new(10, 0) }.should raise_error(IO::Buffer::AllocationError, "Could not allocate buffer!") | ||
| end | ||
andrykonchin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ruby_version_is "3.3" do | ||
| it "raises ArgumentError if flags is negative" do | ||
| -> { IO::Buffer.new(10, -1) }.should raise_error(ArgumentError, "Flags can't be negative!") | ||
| end | ||
| end | ||
|
|
||
| ruby_version_is ""..."3.3" do | ||
| it "raises IO::Buffer::AllocationError with non-Integer flags" do | ||
| -> { IO::Buffer.new(10, 0.0) }.should raise_error(IO::Buffer::AllocationError, "Could not allocate buffer!") | ||
| end | ||
| end | ||
|
|
||
| ruby_version_is "3.3" do | ||
| it "raises TypeError with non-Integer flags" do | ||
| -> { IO::Buffer.new(10, 0.0) }.should raise_error(TypeError, "not an Integer") | ||
| end | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: It seems to me that this logic is closer to the constructor methods specs (new/map/for) than to the predicates. I would keep here just a couple of examples when a predicate returns either true or false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean, but not sure what to even leave here then, especially when many cases of these flags are not intuituve. Hmm, maybe leave these specs as-is for now, but then simplify them when we have specs for all constructors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mentioned above as for me it makes sense to have there a test case when a predicate returns true and a test case with false.
Yeah, it's OK to not make changes in this PR if you are going to add specs for constructors in a separate one.