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

Probe branch: Failing test for missing probe. #13

Closed
wants to merge 1 commit into from

Conversation

rklancer
Copy link

I noticed that the first initializer in var b = 2, c = 3 isn't probed. tracer.traceProbeValue is actually called but the corresponding nodeId doesn't exist in the list of nodeIds sent to tracer.add, so it seems to be dropped.

I saw that you implied work on nodeIds wasn't quite complete, and I'm still working out how Fondue works well enough to contribute a patch, but I was able to at least produce this failing test to demonstrate the problem. I'm offering it as this PR if it's useful to you.

@alltom
Copy link
Member

alltom commented Jan 12, 2014

Great! The tests were already passing again as of 4a731a9, but I hadn't realized the tap functions take an explanation of the failure as the third argument. That will help make them much more clear...

The probes being dropped are part of the current design because my visualization only shows the value for the right-most expression on a line. The change to keep all of them would be simple (instead of appending them to the nodes list at the end, add all of them as the AST is walked) and I'd be willing to make/accept that change if that's what you need. It seems from the discussion in #11 that you might need a little more than that. We should hash that out first!

@alltom alltom closed this Jan 12, 2014
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.

2 participants