test rendering and fix non-determinism #118
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The rendering code keeps breaking across changes made by different contributors. Thus, we needed to test the rendering code too. We do this in a headless fashion and compare the textual 'dot' representation of the rendered SCFG that the graphviz package produces to a predefined, expected result. Since this type of test is rather verbose, we minimize the amount of testing to simply exercise some of the core rendering routines. However, this test exposed multiple bugs across the code-base: * The if-else in the core rendering was broken and no longer worked for simple SCFGs. * The `render_scfg` function was completely broken, since the rendering is now done in the constructors and the `render_scfg` only needs to return the graphviz digraph. * The `dot` output itself was non-deterministic. That is to say, both the order of the nodes in the SCFG itself and also the order of the nodes in the `dot` output were unpredictable. This is due to the use the of the Python set class, which doesn't have order and when iterating over a set the elements will be returned in a random order. Graphviz doesn't care about order, so the rendered SCFGs would look largely the same, which is perhaps why this went unnoticed. Also, sometimes isomorphic variants would be produced, that are semantically equivalent but had different block ordering. As result several calls to `sorted` and use of a `deque` were inserted to stabilize both the order of the blocks within the SCFG datastructure and also the creation of an SCFG from a YAML representation.
Closed
kc611
approved these changes
May 13, 2024
yeah, will have to keep an eye on it and make some changes if it turns out to be too unstable. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The rendering code keeps breaking across changes made by different contributors. Thus, we needed to test the rendering code too. We do this in a headless fashion and compare the textual 'dot' representation of the rendered SCFG that the graphviz package produces to a predefined, expected result. Since this type of test is rather verbose, we minimize the amount of testing to simply exercise some of the core rendering routines.
However, this test exposed multiple bugs across the code-base:
The if-else in the core rendering was broken and no longer worked for simple SCFGs.
The
render_scfgfunction was completely broken, since the rendering is now done in the constructors and therender_scfgonly needs to return the graphviz digraph.The
dotoutput itself was non-deterministic. That is to say, both the order of the nodes in the SCFG itself and also the order of the nodes in thedotoutput were unpredictable. This is due to the use the of the Python set class, which doesn't have order and when iterating over a set the elements will be returned in a random order. Graphviz doesn't care about order, so the rendered SCFGs would look largely the same, which is perhaps why this went unnoticed. Also, sometimes isomorphic variants would be produced, that are semantically equivalent but had different block ordering. As result several calls tosortedand use of adequewere inserted to stabilize both the order of the blocks within the SCFG datastructure and also the creation of an SCFG from a YAML representation.