Skip to content
This repository has been archived by the owner on Apr 9, 2019. It is now read-only.

Robase hsm #6

Merged
merged 7 commits into from
Dec 6, 2018
Merged

Robase hsm #6

merged 7 commits into from
Dec 6, 2018

Conversation

ckdavid
Copy link

@ckdavid ckdavid commented Nov 1, 2018

No description provided.

@ckdavid ckdavid requested review from Hugheth and inversion November 1, 2018 18:59
@ckdavid ckdavid changed the base branch from master to dev/0.1.0 November 1, 2018 18:59
@ckdavid ckdavid force-pushed the robase-hsm branch 2 times, most recently from c28b79a to b0d6058 Compare November 2, 2018 11:02
@@ -0,0 +1,121 @@
local Fsm = require(script.Parent.Fsm)
Copy link

Choose a reason for hiding this comment

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

Is this the standard convention for Robase now? I seem to remember we were evaluating baste at one point

Copy link
Author

Choose a reason for hiding this comment

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

We can use Roblox requires in lemur tests, so I don't see a downside.

lib/Hsm.lua Outdated
hsm,
{
__index = function(_, key)
if Hsm[key] then
Copy link

Choose a reason for hiding this comment

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

return Hsm[key] or hsmEvent(key)?

Copy link

Choose a reason for hiding this comment

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

Is it right to be polluting the Hsm namespace with arbitrary name bindings?

Copy link
Author

Choose a reason for hiding this comment

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

It is assumed that the names are events, and if there is no such event, it will resolve to nil, so calling the event will cause an error, as for the underlying fsm library.

initial = {state = initial}
elseif not initial then
initial = {state = "none"}
end
Copy link

Choose a reason for hiding this comment

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

final else? assert?

@@ -50,7 +50,7 @@ luac.out
/robase-venv

# JUnit test report
/testReport.xml
Copy link

Choose a reason for hiding this comment

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

?

Copy link
Author

Choose a reason for hiding this comment

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

The output file for test runs has been renamed in the Jenkinsfile.

@@ -44,7 +44,12 @@ pipeline {

stage('Tests') {
steps {
sh 'rm -f luacov.stats.* luacov.report.* testReport.xml cobertura.xml && ./test.sh --verbose --coverage --output junit > testReport.xml && ./lua_install/bin/luacov-cobertura -o cobertura.xml'
sh '''
Copy link

Choose a reason for hiding this comment

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

what does this change do?

Copy link
Author

Choose a reason for hiding this comment

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

Rather than redirecting the test output, which includes print statements, it uses -Xoutput to pass in the name of the output file. It also strips whitespace lines from the report.

@ckdavid ckdavid merged commit d6ba354 into dev/0.1.0 Dec 6, 2018
@ckdavid ckdavid deleted the robase-hsm branch December 6, 2018 12:19
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.

2 participants