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

doesNotUseRecursion fix #947

Merged
merged 8 commits into from
Apr 13, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions app/utils/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const multiExpect = (...expectations) => (element) =>

// DECLARATION EXPECTATIONS
export const doSomething = (declaration) =>
newExpectation(`within ${toEDLString(declaration)} count(calls) >= 1`, 'do_something', { declaration })
newExpectation(`${countWithRecursiveCalling(declaration)} >= 1`, 'do_something', { declaration })

export const isUsed = (declaration) =>
newExpectation(`calls ${toEDLString(declaration)}`, 'is_used', { declaration })
Expand All @@ -21,15 +21,18 @@ export const isUsedFromMain = (declaration) =>
newExpectation(`through ${toEDLString(entryPointType)} calls ${toEDLString(declaration)}`, 'is_used_from_main', { declaration })

export const notTooLong = (limit = 7) => (declaration) =>
newExpectation(`within ${toEDLString(declaration)} count(calls) <= ${limit - 1}`, 'too_long', { declaration, limit })
newExpectation(`${countWithRecursiveCalling(declaration)} <= ${limit - 1}`, 'too_long', { declaration, limit })

export const doesNotUseRecursion = (declaration) =>
newExpectation(`through ${toEDLString(declaration)} ! calls ${toEDLString(declaration)}`, doesNotUseRecursionId, { declaration })
newExpectation(`not (through ${toEDLString(declaration)} calls ${toEDLString(declaration)})`, doesNotUseRecursionId, { declaration })

// UTILS
const newExpectation = (expect, id, opts = {}) =>
`expectation "${stringify(id, opts)}": ${expect};`

const countWithRecursiveCalling = (declaration) =>
`within ${toEDLString(declaration)} count(calls) + count(calls ${toEDLString(declaration)})`

asanzo marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Debería llamarse countCallsWithin y adentro tener un comentario que explique este problema

Copy link
Contributor

Choose a reason for hiding this comment

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

Cargar pregunta/issue en mulang ¿por qué el count(calls) no incluye las recursivas?

Copy link
Contributor

Choose a reason for hiding this comment

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

export const stringify = (id, opts) =>
`${expectationName(id)}|${Object.entries(opts).map(([key, value]) => `${key}=${value}`).join(';')}`

Expand Down
50 changes: 36 additions & 14 deletions tests/unit/services/mulang-expectations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ module('Unit | Service | Mulang | Expectations', function (hooks) {
application('PRIMITIVE')
)
])


expectationTestOk('doSomething', doSomething(declaration), [
procedure(declaration, [],
application(declaration)
)
], 'Recursion should count as doing something')

expectationTestFail('doSomething', doSomething('EMPTY'), [
procedure('EMPTY', [])
])
Expand Down Expand Up @@ -84,42 +90,58 @@ module('Unit | Service | Mulang | Expectations', function (hooks) {
application('PRIMITIVE'),
)
])


expectationTestFail('notTooLong', notTooLong(limit)(declaration), [
procedure(declaration, [],
application(declaration),
application(declaration),
application(declaration)
)
], 'Recursive calls should count as being too long ')

expectationTestOk('doesNotUseRecursion', doesNotUseRecursion(declaration), [
procedure(declaration, [],
application("PROCEDURE2")
application("PROCEDURE2")
),
procedure("PROCEDURE2", [])
])

// Direct recursion
expectationTestFail('doesNotUseRecursion', doesNotUseRecursion(declaration), [
procedure(declaration, [],
application(declaration)
application(declaration)
)
])
/*

// Indirect recursion
expectationTestFail('doesNotUseRecursion', doesNotUseRecursion(declaration), [
procedure(declaration, [],
application("PROCEDURE2")
application("PROCEDURE2")
),
procedure("PROCEDURE2", [],
application(declaration)
)
])
*/
], 'Indirect recursion should count as recursion')

expectationTestFail('doesNotUseRecursion', doesNotUseRecursion(declaration), [
procedure(declaration, [],
application(declaration),
application("PROCEDURE2")
),
procedure("PROCEDURE2", [],
application('PRIMITIVE'))
], 'Direct recursion with another procedure call should count as recursion')

function expectationTestOk(expectationName, expectation, astNodes) {
expectationTest(expectationName, expectation, astNodes, true)
function expectationTestOk(expectationName, expectation, astNodes, testName) {
expectationTest(expectationName, expectation, astNodes, true, testName)
}

function expectationTestFail(expectationName, expectation, astNodes) {
expectationTest(expectationName, expectation, astNodes, false)
function expectationTestFail(expectationName, expectation, astNodes, testName) {
expectationTest(expectationName, expectation, astNodes, false, testName)
}

function expectationTest(expectationName, edl, astNodes, shouldPass) {
test(`Expectation ${expectationName} - ${shouldPass ? 'ok' : 'fail'}`, function (assert) {
function expectationTest(expectationName, edl, astNodes, shouldPass, testName = '') {
test(`Expectation ${expectationName} - ${testName || (shouldPass ? 'ok' : 'fail')}`, function (assert) {
const mulangResult = mulang
.astCode(rawSequence(astNodes))
.customExpect(edl)
Expand Down