Skip to content

String utils updates and tests#3434

Merged
demonnic merged 5 commits intoMudlet:developmentfrom
demonnic:string_utils_tests
Mar 13, 2020
Merged

String utils updates and tests#3434
demonnic merged 5 commits intoMudlet:developmentfrom
demonnic:string_utils_tests

Conversation

@demonnic
Copy link
Copy Markdown
Member

Brief overview of PR changes/additions

While writing the tests for StringUtils I realized it would only take a bit of adjustment to support the : notation for the functions we add.

  • So I added that functionality.
  • I also adjusted the error message for string.title to be more informative/match other errors
  • added tests for all the functions in StringUtils.lua which also ensured the functional changes didn't break anything. At least one test for each functions tests both . and : notation.

Motivation for adding to Mudlet

TEST ALL THE THINGS

Other info (issues closed, discussion etc)

image

@demonnic demonnic requested a review from a team March 13, 2020 02:41
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Mar 13, 2020

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

Copy link
Copy Markdown
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

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

Looks great 😄

function string:title()
assert(type(self) == "string", "string.title(): no word given to capitalize")
local strType = type(self)
assert(strType == "string", "string.title(str): str as string expected, got " .. strType)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you use the C++ variant of the error msg for consistency?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok. Because consistency is good. But I still don't like it. =)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I had no idea you don't like it! Feel free to raise it for discussion.

function string:title()
local strType = type(self)
assert(strType == "string", "string.title(str): str as string expected, got " .. strType)
assert(strType == "string", string.format("string.title: bad argument #1 type. (String to title as string expected, got %s!)", strType))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
assert(strType == "string", string.format("string.title: bad argument #1 type. (String to title as string expected, got %s!)", strType))
assert(strType == "string", string.format("string.title: bad argument #1 type (string to title as string expected, got %s!)", strType))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

doh. fixed.

local str = {}
local errfn = function() string.title(str) end
assert.has_error(errfn, "string.title(str): str as string expected, got table")
assert.has_error(errfn, "string.title: bad argument #1 type. (String to title as string expected, got table!)")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
assert.has_error(errfn, "string.title: bad argument #1 type. (String to title as string expected, got table!)")
assert.has_error(errfn, "string.title: bad argument #1 type (string to title as string expected, got table!)")

@demonnic demonnic merged commit 4b9bb85 into Mudlet:development Mar 13, 2020
@demonnic demonnic deleted the string_utils_tests branch March 13, 2020 18:58
@demonnic demonnic linked an issue Mar 13, 2020 that may be closed by this pull request
Chris7 pushed a commit to Chris7/Mudlet that referenced this pull request Jan 2, 2022
* Adjusting items in StringUtils to support the : function notation
* Adding StringUtils spec, to make sure that the changes I made didn't break anything
* Adjusting one test and adding one last one
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.

Add busted (or luaunit) tests as part of CI

2 participants