Skip to content
This repository has been archived by the owner on Aug 4, 2024. It is now read-only.

Import test refcount #249

Merged
merged 4 commits into from
Mar 4, 2023
Merged

Conversation

wolfsage
Copy link
Contributor

This way we have a base to start from to understand changes
Test2-Suite maybe already has one? And if not, this isn't the
place to add one, probably.
Test2 recommends against it, and failure to compile will be
caught by the other tests
@wolfsage
Copy link
Contributor Author

@leonerd This is almost a direct import of Test-Refcount. I've removed some tests that I don't think make sense to port over, and converted the rest of the tests and the module to use Test2::API etc...

Is this roughly what you imagined? I have not touched documentation, etc.

I'm not sure if you want identical modules with different code being maintained separately or not, or if one should point to the other, copyrights, so on and so forth...

@leonerd
Copy link
Contributor

leonerd commented Aug 31, 2022

Is this roughly what you imagined? I have not touched documentation, etc.

I'm really not sure. It's an almost-exact copy of my original module so clearly it has the ability to test the same things I was looking for. But I can't speak for whether it's a good cultural fit for your ecosystem - in terms of names, argument types, operating style, etc... That was really the reason I didn't want to write it. I don't know what would fit with the rest of Test2

@wolfsage
Copy link
Contributor Author

I did it this way so if both modules coexist, it should be easy to port fixes from one to the other since they look almost identical.

The other option is to maybe build Test::Refcount on top of Test2 and drop the Test::Builder, but then it won't be usable on any Test::More that predates the Test2 compatibility layer...


=head1 SYNOPSIS

use Test::More tests => 2;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe for this replace Test::More with Test2::V0 or similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Yes I did not touch the documentation basically at all because I wasn't sure how this was all going to look at the end :))

@exodist
Copy link
Member

exodist commented Aug 31, 2022

The main docs refer to Test::More, it should probably be Test2::V0 instead for this PR.
The provided tools are fine
I would also maybe add an optional export for refcount(). I think in Test2 the most likely usage would be:

is(refcount($foo), 5);   but I am fine with is_refcount() or refcount_is() also existing.

is_oneref() is very good to have too.

@wolfsage
Copy link
Contributor Author

Thanks for the review, will try to find time for that soon

@exodist exodist merged commit 8ab00d0 into Test-More:master Mar 4, 2023
@wolfsage
Copy link
Contributor Author

wolfsage commented Mar 4, 2023

Hey thanks!

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

Successfully merging this pull request may close these issues.

3 participants