swaroopch / edn_format

EDN reader and writer implementation in Python, using PLY (lex, yacc)
https://www.swaroopch.com/Wrote-an-EDN-format-reader-and-writer-in-Python-11e0924249b181e3af8bdb9af1456373
Other
134 stars 31 forks source link

[PATCH] Failing test for set round-tripping #21

Closed rmunn closed 10 years ago

rmunn commented 10 years ago

One of the round-trip tests failed the first time I ran it:

rmunn@laptop:~/code/python/edn_format (master)$ python tests.py 
......F.
======================================================================
FAIL: test_round_trip_conversion (__main__.EdnTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests.py", line 140, in test_round_trip_conversion
    self.assertIn(step3, literal[1])
AssertionError: '#{:b (1 2 3) :a}' not found in ['#{:a (1 2 3) :b}', '#{(1 2 3) :a :b}', '#{:a :b (1 2 3)}', '#{:b :a (1 2 3)}']

----------------------------------------------------------------------
Ran 8 tests in 0.447s

FAILED (failures=1)

There are six possible permutations of :a, :b, and (1 2 3), but only four of those permutations are checked for in the test results. The following patch will check for the other two possible permutations:

diff --git a/tests.py b/tests.py
index df26228..6a272ab 100644
--- a/tests.py
+++ b/tests.py
@@ -126,7 +126,9 @@ class EdnTest(unittest.TestCase):
             ["+32.23M", "32.23M"],
             ["3.23e10", "32300000000.0"],
             ['#{:a (1 2 3) :b}', ['#{:a (1 2 3) :b}',
+                                  '#{:b (1 2 3) :a}',
                                   '#{(1 2 3) :a :b}',
+                                  '#{(1 2 3) :b :a}',
                                   '#{:a :b (1 2 3)}',
                                   '#{:b :a (1 2 3)}']]
         ]

The patch is simple enough that I'm not going to bother with setting up a pull request, though I can do so if you'd prefer.

bitemyapp commented 10 years ago

@swaroopch @rmunn the test is nonsensical period, you can't guarantee ordering with hash-array mapped tries and that's what the test is testing. It'll fail randomly because of the non-deterministic ordering.

The test has to be predicated on set membership rather than ordering to:

  1. Make sense
  2. Work reliably
swaroopch commented 10 years ago

@bitemyapp I agree, hope https://github.com/swaroopch/edn_format/commit/e04475aa2bbcdcd07813c6f129f43db289bedeb4 makes more sense.

bitemyapp commented 10 years ago

@swaroopch yeah I think that should fix the intermittent test failures related to order dependency for that test! Thanks :D

swaroopch commented 10 years ago

@bitemyapp Thanks for keeping watch!