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

Add tests for trim* subroutines on Cool instance #324

Merged
merged 3 commits into from Sep 18, 2017
Merged

Add tests for trim* subroutines on Cool instance #324

merged 3 commits into from Sep 18, 2017

Conversation

cuonglm
Copy link
Contributor

@cuonglm cuonglm commented Sep 18, 2017

No description provided.

@lizmat
Copy link
Contributor

lizmat commented Sep 18, 2017

You should really consider Cool a role. In my opinion, it just isn't because of historical reasons.

A Cool.new instance is without meaning. So the tests as such as really without meaning, even though they pass. It probably makes more sense to test for Ints, since these are also Cool but not Str.

So, my $c = 42 instead of my $c = Cool.new would do the trick.

@cuonglm
Copy link
Contributor Author

cuonglm commented Sep 18, 2017

@lizmat fixed it.

PS: Why I see number 42 is often used in tests? Is there any reason?

@zoffixznet
Copy link
Contributor

zoffixznet commented Sep 18, 2017

It probably makes more sense to test for Ints,

IO::Path is a better candidate, since you can actually test whether trimming works.

Also, were these tests to fail, the failure would be rather non-descriptive. It'd be nicer to test actual result instead of relying on behaviour of other methods

plan 1;
subtest 'trim routines on Cool' => {
    plan 6;
    my $p = ' foo '.IO;
    is-deeply trim($p),           'foo',  '&trim';
    is-deeply trim-leading($p),   'foo ', '&trim-leading';
    is-deeply trim-trailing($p), ' foo',  '&trim-trailing';

    is-deeply $p.trim,            'foo',  '.trim';
    is-deeply $p.trim-leading,    'foo ', '.trim-leading';
    is-deeply $p.trim-trailing,  ' foo',  '.trim-trailing';
}

Now, if some routine returns incorrect result, you'll actually see the wanted/expected values and why it failed.

@zoffixznet zoffixznet merged commit c1d8794 into Raku:master Sep 18, 2017
@zoffixznet
Copy link
Contributor

👍 Thanks!

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.

None yet

3 participants