-
Notifications
You must be signed in to change notification settings - Fork 44
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
Feat: Add testing for utils & delete extraneous code; #145
Merged
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
1f04c6d
feat(tests): separate test modules;
5-pebbles cda0836
feat(tests): add assert_eq to test module;
5-pebbles 5f3658a
ci(tests): echo tests output on complete;
5-pebbles 2261642
fix(tests): remove intentional failures;
5-pebbles c117dae
feat(tests): assert_eq works on tables now;
5-pebbles 21c2ca9
feat(tests): add tests for none, is_none, & hex_to_rgb;
5-pebbles e7d742a
feat(tests): add tests & fix merge;
5-pebbles f5ff09c
fix(tests): fix bad module name in error messages;
5-pebbles 289870e
feat(tests): add tests for merge_inplace;
5-pebbles 706f874
fix(utils): merge_inplace clones values in t1 nested tables;
5-pebbles 7c38ee1
feat(utils): add is_table to utils;
5-pebbles ae12e29
feat(tests): add tests for rgb_to_hex;
5-pebbles be45c60
fix(utils): rgb_to_hex returns uppercase like everything else;
5-pebbles a109c94
feat(tests): readable table asserts;
5-pebbles a741e90
feat(tests): add tests for rgb_to_hsv;
5-pebbles e627217
fix(utils): hsv_to_rgb function was misnamed;
5-pebbles ea111b0
feat(tests): add tests for hsv_to_rgb;
5-pebbles 7876f49
rm(utils): rgb_to_hsv, hsv_to_rgb, darken, & lighten;
5-pebbles 9d89242
rm(utils): replace merge with merge_inplace;
5-pebbles 3349c8b
feat(tests): add tests for blend;
5-pebbles 4f5b445
feat(tests): add test for merge_inplace returning a reference;
5-pebbles 8c8fff4
feat(utils): re-add merge & cleanup merge_inplace usage;
5-pebbles 4bcea92
fix(tests): replace pretty_string with vim.inspect();
5-pebbles 34f5427
fix(tests): replace recursive_eq with vim.deep_equal();
5-pebbles ce6062a
ref(tests): cleanup and organising
AlexvZyl e10e668
ref(tests): improve equal assert message
AlexvZyl 265ba41
ref(tests): Correct test...
AlexvZyl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
local U = require('nordic.utils') | ||
|
||
M = {} | ||
|
||
local function pretty_string(value) | ||
AlexvZyl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if not U.is_table(value) then return tostring(value) end | ||
local output = '{' | ||
for k, v in pairs(value) do | ||
output = output .. '' .. tostring(k) .. ': ' .. pretty_string(v) .. ', ' | ||
end | ||
if #output == 1 then | ||
return output .. '}' | ||
else | ||
return output:sub(1, -3) .. '}' | ||
end | ||
end | ||
|
||
function M.assert_eq(left, right, message) | ||
AlexvZyl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
local function recursive_eq(value1, value2) | ||
if not U.is_table(value1) or not U.is_table(value2) then | ||
return value1 == value2 | ||
end | ||
|
||
-- I think this can be done faster but its just for tests... | ||
AlexvZyl marked this conversation as resolved.
Show resolved
Hide resolved
AlexvZyl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for k, v in pairs(value1) do | ||
if not recursive_eq(v, value2[k]) then return false end | ||
end | ||
for k, v in pairs(value2) do | ||
if not recursive_eq(v, value1[k]) then return false end | ||
end | ||
return true | ||
end | ||
|
||
if not recursive_eq(left, right) then | ||
local info = debug.getinfo(2) | ||
local file_name = info.short_src | ||
local line_number = info.currentline | ||
print('\nassertion `left == right` failed' .. | ||
((message and ': ' .. message) or '') .. | ||
'\n left: ' .. | ||
pretty_string(left) .. | ||
'\n right: ' .. pretty_string(right) .. '\nat ' .. tostring(file_name) .. ':' .. tostring(line_number)) | ||
end | ||
end | ||
|
||
function M.run_tests() | ||
require('nordic.tests.options') | ||
require('nordic.tests.utils') | ||
end | ||
|
||
return M |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
local assert_eq = require('nordic.tests').assert_eq | ||
local U = require('nordic.utils') | ||
|
||
-- none | ||
assert_eq(U.none(), 'NONE', 'utils.none() should return "NONE"') | ||
|
||
-- is_none | ||
assert_eq(U.is_none('NONE'), true, 'U.is_none("NONE") should return true') | ||
assert_eq(U.is_none('none'), true, 'U.is_none("none") should return true') | ||
assert_eq(U.is_none('nil'), false, 'U.is_none("nil") should return false') | ||
|
||
-- is_table | ||
assert_eq(U.is_table('string'), false, 'U.is_table("string") should return false') | ||
assert_eq(U.is_table(4), false, 'U.is_table(4) should return false') | ||
assert_eq(U.is_table({}), true, 'U.is_table({}) should return true') | ||
|
||
-- merge_inplace | ||
local t1 = {a = 1} | ||
local t2 = {b = 2} | ||
U.merge_inplace(t1, t2) | ||
assert_eq(t1, {a = 1, b = 2}, 'U.merge_inplace(t1, t2) basic merge') | ||
|
||
local t1 = {a = 1} | ||
local t2 = {b = 2} | ||
assert_eq(U.merge_inplace(t1, t2) == t1, true, 'U.merge_inplace(t1, t2) should return t1') | ||
|
||
local t1 = {a = 1, b = 3} | ||
local t2 = {b = 2, c = 4} | ||
U.merge_inplace(t1, t2) | ||
assert_eq(t1, {a = 1, b = 2, c = 4}, 'U.merge_inplace(t1, t2) overwriting values') | ||
|
||
local t1 = {a = 1, d = {x = 10}} | ||
local t2 = {d = {y = 20}, e = 5} | ||
U.merge_inplace(t1, t2) | ||
assert_eq(t1, {a = 1, d = {x = 10, y = 20}, e = 5}, 'U.merge_inplace(t1, t2) nested tables') | ||
|
||
local nested = { y = 20 } | ||
local t1 = {d = nested, e = 5} | ||
local t2 = {a = 1} | ||
U.merge_inplace(t1, t2) | ||
assert_eq(t1['d'] ~= nested, true, 'U.merge_inplace(t1, t2) copy t1 nested values') | ||
|
||
local nested = { y = 20 } | ||
local t1 = {a = 1} | ||
local t2 = {d = nested, e = 5} | ||
U.merge_inplace(t1, t2) | ||
assert_eq(t1['d'] ~= nested, true, 'U.merge_inplace(t1, t2) copy t2 nested values') | ||
|
||
-- hex_to_rgb | ||
assert_eq({ U.hex_to_rgb('#191D24') }, {25, 29, 36}, 'U.hex_to_rgb("#191D24") should return 25, 29, 36') | ||
|
||
-- rgb_to_hex | ||
assert_eq(U.rgb_to_hex(25, 29, 36), '#191D24', 'U.rgb_to_hex(25, 29, 36) should return "#191D24"') | ||
|
||
-- blend | ||
assert_eq(U.blend('#FFFFFF', '#000000', 0.5), '#808080', 'U.blend("#FFFFFF", ""#000000", 0.5) should return "#808080"') |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Pretty sure this should not be a merge inplace? We do not want to modify
native
here.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.
It works because
get_groups
creates a new table each time; however, I agree it's not very clear.merge
andmerge_inplace
do almost the same thing, so it feels weird to have both.I could revert that commit or I could make it more clear with something like this:
Which ever you think is better.
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.
Merge and merge inplace are different enough that we would want both, it is very common to have both.
So I think we should:
merge_inplace
in this scenario, please change it the way you suggested. Just a bit more clear what is going on.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.
done in: 8c8fff4