Implemented optional data validation by Masken8 · Pull Request #118 · Kampfkarren/Roblox

Masken8 · 2021-01-01T18:09:32Z

Implemented #64

Kampfkarren

This needs a unit test as well.

DataStore2/init.lua

Tests/tests/DataStore2.spec.lua

Kampfkarren

Sorry for giving you a lot of work, just want to make sure that the DataStore2 codebase is as solid as can be.

Tests/tests/DataStore2.spec.lua

DataStore2/init.lua

Tests/tests/DataStore2.spec.lua

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Kampfkarren

You can click "Add suggestion to batch" to combine them all into one commit, by the way.

DataStore2/init.lua

Tests/tests/DataStore2.spec.lua

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Kampfkarren · 2021-01-01T23:26:45Z

Can you please provide proof that you ran the tests and they pass? I'm trying to get them running on CI.

Kampfkarren · 2021-01-01T23:38:04Z

Tests run on CI now, merge from latest master.

…alidation

Kampfkarren · 2021-01-02T02:02:04Z

Ugh, that specific error is not your fault. I'll look into it.

Kampfkarren · 2021-01-02T02:04:10Z

I think I don't have the ability to run tests on PRs since you don't have the authentication token, I'm going to clone your branch and test it on there.

Kampfkarren · 2021-01-02T02:11:30Z

Your tests failed--#121

Please make a good attempt to run the tests locally.

All it should take is:

Download the submodules (for TestEZ and MockDataStoreWrapper)
Build tests.project.json into an rbxlx
Open that rbxlx, and click Run
See the output

Masken8 · 2021-01-02T14:35:16Z

I'm getting $path referred to a path that could not be turned into an instance by Rojo and then Rojo crashes

Kampfkarren · 2021-01-02T23:53:45Z

I'd recommend doing a bisect, removing paths from the project.json until it works, so you know which path in specific is causing the issues. Make sure your Rojo is up to date as well.

I know what is causing the test failures, but I want to make sure you know how to run the tests yourself!

Masken8 · 2021-01-03T00:59:27Z

Attempted to set data store to an invalid value during :Set

Attempted to set data store to an invalid value during :Update

TestService: ServerScriptService.Tests.Tests.DataStore2.spec:96: Expected function to throw an error containing "Attempted to set data store to an invalid value during :Set", but it threw: attempt to call a table value

ServerScriptService.Tests.TestEZ.Expectation:47 function assertLevel
ServerScriptService.Tests.TestEZ.Expectation:305 function throw
ServerScriptService.Tests.TestEZ.Expectation:59
ServerScriptService.Tests.Tests.DataStore2.spec:96
ServerScriptService.Tests.TestEZ.TestRunner:86
ServerScriptService.Tests.TestEZ.TestRunner:84 function runCallback
ServerScriptService.Tests.TestEZ.TestRunner:116 function runNode
ServerScriptService.Tests.TestEZ.TestRunner:153 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:164 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:164 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:164 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:164 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:45 function runPlan
ServerScriptService.Tests.TestEZ.TestBootstrap:127 function run
ServerScriptService.Tests.Tests.TestRunner:5

TestService: ServerScriptService.Tests.Tests.DataStore2.spec:116: Expected function to throw an error containing "Attempted to set data store to an invalid value during :Update", but it threw: attempt to call a nil value

ServerScriptService.Tests.TestEZ.Expectation:47 function assertLevel
ServerScriptService.Tests.TestEZ.Expectation:305 function throw
ServerScriptService.Tests.TestEZ.Expectation:59
ServerScriptService.Tests.Tests.DataStore2.spec:116
ServerScriptService.Tests.TestEZ.TestRunner:86
ServerScriptService.Tests.TestEZ.TestRunner:84 function runCallback
ServerScriptService.Tests.TestEZ.TestRunner:116 function runNode
ServerScriptService.Tests.TestEZ.TestRunner:153 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:164 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:164 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:164 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:164 function runPlanNode
ServerScriptService.Tests.TestEZ.TestRunner:45 function runPlan
ServerScriptService.Tests.TestEZ.TestBootstrap:127 function run
ServerScriptService.Tests.Tests.TestRunner:5

How, I cannot for the life of me figure this out.

Edit: Might have figured it out
Edit 2: or not

Kampfkarren · 2021-01-03T01:26:17Z

Request review from me when you have something that works so I can run it on the CI runner.

Masken8 · 2021-01-09T18:08:11Z

I have been trying to figure this out for a week now and I'm still none the wiser. I'm still stuck with the cryptic error messages above. The same code without the unit testing works completely fine though so the errors must be caused by TestEZ somehow 🤔

Kampfkarren · 2021-01-09T18:19:02Z

Tests/tests/DataStore2.spec.lua

+				end
+
+				dataStore:SetValidator(testValidator)
+				expect(dataStore:Set("nope")).to.throw("Attempted to set data store to an invalid value during :Set")


This isn't how .to.throw works, if dataStore:Set("nope") errors then it would error the entire script!

You need the function there because that's what's actually pcalled. The way you have it now looks like :Set returns a function that you're checking.

Added optional validation to DataStore:Set and DataStore:Update

2cb36f0

Masken8 changed the title ~~Implemented optional data validation (#64)~~ Implemented optional data validation #64 Jan 1, 2021

Masken8 changed the title ~~Implemented optional data validation #64~~ Implemented optional data validation Jan 1, 2021

Kampfkarren requested changes Jan 1, 2021

View reviewed changes

DataStore2/init.lua Outdated Show resolved Hide resolved

DataStore2/init.lua Outdated Show resolved Hide resolved

DataStore2/init.lua Outdated Show resolved Hide resolved

Masken8 added 2 commits January 1, 2021 21:54

Change 2

21fd0e5

Added request 1 and 3

c6fecb1

Kampfkarren requested changes Jan 1, 2021

View reviewed changes

DataStore2/init.lua Outdated Show resolved Hide resolved

DataStore2/init.lua Outdated Show resolved Hide resolved

DataStore2/init.lua Outdated Show resolved Hide resolved

DataStore2/init.lua Outdated Show resolved Hide resolved

DataStore2/init.lua Outdated Show resolved Hide resolved

Masken8 added 4 commits January 1, 2021 23:10

Added unit test for validation

e25255a

assertValidatorWithDefaultError no longer uses pcall

c12c18f

Fixed unit test

d808ebb

Edited error messages

81a1d81

Kampfkarren reviewed Jan 1, 2021

View reviewed changes

DataStore2/init.lua Outdated Show resolved Hide resolved

Kampfkarren reviewed Jan 1, 2021

View reviewed changes

Tests/tests/DataStore2.spec.lua Outdated Show resolved Hide resolved

Kampfkarren reviewed Jan 1, 2021

View reviewed changes

Tests/tests/DataStore2.spec.lua Outdated Show resolved Hide resolved

Masken8 added 2 commits January 1, 2021 23:59

Validator can return a message

f2e065b

Fixed validate set unit test and added validate update unit test

903f0d8

Kampfkarren requested changes Jan 1, 2021

View reviewed changes

Masken8 and others added 8 commits January 2, 2021 00:09

Update Tests/tests/DataStore2.spec.lua

4d9cd6c

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Update Tests/tests/DataStore2.spec.lua

d79c4bb

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Update DataStore2/init.lua

2310328

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Update DataStore2/init.lua

998df5c

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Update DataStore2/init.lua

b336b08

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Update DataStore2/init.lua

d36a7fb

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Whitespace

26579c4

Validators in unit tests no longer error themselves

85b221b

Kampfkarren requested changes Jan 1, 2021

View reviewed changes

Apply suggestions from code review

7d80f3d

Co-authored-by: boyned//Kampfkarren <boynedmaster@gmail.com>

Merge branch 'master' of https://github.com/Kampfkarren/Roblox into V…

9589023

…alidation

Kampfkarren mentioned this pull request Jan 2, 2021

Ignore this PR, just for running tests #121

Closed

Unit test fixed for real this time I think

1f2b37e

Kampfkarren reviewed Jan 9, 2021

View reviewed changes

Conversation

Masken8 commented Jan 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kampfkarren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kampfkarren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kampfkarren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kampfkarren commented Jan 1, 2021

Uh oh!

Kampfkarren commented Jan 1, 2021

Uh oh!

Kampfkarren commented Jan 2, 2021

Uh oh!

Kampfkarren commented Jan 2, 2021

Uh oh!

Kampfkarren commented Jan 2, 2021

Uh oh!

Masken8 commented Jan 2, 2021

Uh oh!

Kampfkarren commented Jan 2, 2021

Uh oh!

Masken8 commented Jan 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kampfkarren commented Jan 3, 2021

Uh oh!

Masken8 commented Jan 9, 2021

Uh oh!

Kampfkarren Jan 9, 2021

Choose a reason for hiding this comment

Uh oh!

Kampfkarren Jan 9, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Masken8 commented Jan 1, 2021 •

edited

Loading

Masken8 commented Jan 3, 2021 •

edited

Loading