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

Test: existingThoughtChange #815

Merged
merged 3 commits into from
Jul 27, 2020
Merged

Conversation

aqkhan
Copy link
Contributor

@aqkhan aqkhan commented Jul 27, 2020

Fixes #803

expect(getThoughts(stateNew, [ROOT_TOKEN]))
.toMatchObject([{ value: 'b', rank: 1 }, { value: 'aa', rank: 0 }])

expect(stateNew.cursor)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cursor does not update upon editing the active thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raineorshine The state.cursor in this case returns the original path, before edit, i.e [ { value: 'a', rank: 0 } ]

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch. It appears that existingThoughtChange does not update the cursor at all. This will be fixed separately.

- ab`)

// ab should exist in both contexts a and b
expect(getContexts(stateNew, 'ab'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getContexts return the context b with rank: 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raineorshine
getContexts in this case returns the following:
[ { context: [ 'a' ], id: '266e7ff0-aa5c-4927-a8a2-bcc2591b8e85', rank: 0 }, { context: [ 'b' ], rank: 0, id: '0783bcd8-c2b8-4e48-9487-a530b3b44be2' } ]
Notice that the b should have rank: 1, but instead it returns rank: 0, which is not desirable.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is correct actually. Since getContexts returns ThoughtContexts not Child nodes, rank represents the rank of the value within each context. i.e. in Context ['a'] the thought ab has rank 0, and in Context ['b'] the thought ab also has rank 0. Ranks are only unique within the same Context.

@raineorshine
Copy link
Contributor

It looks like the tests are failing. Can you provide an update on this? Is this still in progress?

I couldn't tell whether your comments were notes to yourself, or questions, or something else. Thanks!

@aqkhan
Copy link
Contributor Author

aqkhan commented Jul 27, 2020

@raineorshine The comments are notes/reasons for why the tests are failing. Just added more details. The methods does not seem to return the desired values. Let me know how should I proceed. Thanks!

@raineorshine raineorshine merged commit 48778d3 into cybersemics:dev Jul 27, 2020
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.

Test: existingThoughtChange
2 participants