dicts aren't ordered... so converge doesn't happen in order

[altendky]

https://travis-ci.org/Julian/mkenv/jobs/388018638#L534

So the virtualenvs.toml is being loaded into a dict so we can't trust the order on less than Python 3.7 (or 3.6 of course as an implementation detail). Locally for me everything passed fairly consistently. I brought up a local Travis container and mostly got both py27 and py34 failing, but not totally consistently.

The result of one tox run of py27/34 with an added print yielded the dict you might expect to see for a failure.

diff --git a/mkenv/converge.py b/mkenv/converge.py
index bdb9bf2..b50c4fd 100644
--- a/mkenv/converge.py
+++ b/mkenv/converge.py
@@ -39,6 +39,9 @@ def main(filesystem, locator, link_dir, handle_error):
     with filesystem.open(locator.root.descendant("virtualenvs.toml")) as venvs:
         contents = pytoml.load(venvs)
 
+    with open('/home/travis/mkenv/blah', 'a') as f:
+        f.write(str(contents) + '\n')
+
     for name, config in contents["virtualenv"].items():
         config.setdefault("sys.version", sys.version)

{u'virtualenv': {u'a': {}, u'c': {}, u'b': {}, u'magicExplodingVirtualenv': {}}}
{u'virtualenv': {u'a': {u'requirements': [u'requirements.txt'], u'install': [u'foo', u'bar']}}}
{u'virtualenv': {u'a': {u'requirements': [u'requirements.txt', u'other.txt'], u'install': [u'baz', u'quux']}}}
{u'virtualenv': {u'a': {}, u'c': {u'link': [u'bar', u'baz'], u'install': [u'foo', u'$HOME', u'~/a']}, u'b': {u'requirements': [u'requirements.txt'], u'install': [u'foo', u'bar', u'bla']}}}
{u'virtualenv': {u'a': {}, u'c': {}, u'b': {}, u'magicExplodingVirtualenv': {}}}
{'virtualenv': {'magicExplodingVirtualenv': {}, 'a': {}, 'b': {}, 'c': {}}}
{'virtualenv': {'a': {'requirements': ['requirements.txt'], 'install': ['foo', 'bar']}}}
{'virtualenv': {'a': {'requirements': ['requirements.txt', 'other.txt'], 'install': ['baz', 'quux']}}}
{'virtualenv': {'a': {}, 'b': {'requirements': ['requirements.txt'], 'install': ['foo', 'bar', 'bla']}, 'c': {'link': ['bar', 'baz'], 'install': ['foo', '$HOME', '~/a']}}}
{'virtualenv': {'magicExplodingVirtualenv': {}, 'a': {}, 'b': {}, 'c': {}}}

There's a ticket (avakar/pytoml#6) for pytoml to get object_pairs_hook like json. Maybe it's time for me to head over there.

[Julian]

Hm, does something actually rely on them being run in order?

…

On Mon, Jun 4, 2018, 22:23 Kyle Altendorf ***@***.***> wrote: https://travis-ci.org/Julian/mkenv/jobs/388018638#L534 So the virtualenvs.toml is being loaded into a dict so we can't trust the order on less than Python 3.7 (or 3.6 of course as an implementation detail). Locally for me everything passed fairly consistently. I brought up a local Travis container and mostly got both py27 and py34 failing, but not totally consistently. The result of one tox run of py27/34 with an added print yielded the dict you might expect to see for a failure. diff --git a/mkenv/converge.py b/mkenv/converge.py index bdb9bf2..b50c4fd 100644--- a/mkenv/converge.py+++ b/mkenv/converge.py@@ -39,6 +39,9 @@ def main(filesystem, locator, link_dir, handle_error): with filesystem.open(locator.root.descendant("virtualenvs.toml")) as venvs: contents = pytoml.load(venvs) + with open('/home/travis/mkenv/blah', 'a') as f:+ f.write(str(contents) + '\n')+ for name, config in contents["virtualenv"].items(): config.setdefault("sys.version", sys.version) {u'virtualenv': {u'a': {}, u'c': {}, u'b': {}, u'magicExplodingVirtualenv': {}}} {u'virtualenv': {u'a': {u'requirements': [u'requirements.txt'], u'install': [u'foo', u'bar']}}} {u'virtualenv': {u'a': {u'requirements': [u'requirements.txt', u'other.txt'], u'install': [u'baz', u'quux']}}} {u'virtualenv': {u'a': {}, u'c': {u'link': [u'bar', u'baz'], u'install': [u'foo', u'$HOME', u'~/a']}, u'b': {u'requirements': [u'requirements.txt'], u'install': [u'foo', u'bar', u'bla']}}} {u'virtualenv': {u'a': {}, u'c': {}, u'b': {}, u'magicExplodingVirtualenv': {}}} {'virtualenv': {'magicExplodingVirtualenv': {}, 'a': {}, 'b': {}, 'c': {}}} {'virtualenv': {'a': {'requirements': ['requirements.txt'], 'install': ['foo', 'bar']}}} {'virtualenv': {'a': {'requirements': ['requirements.txt', 'other.txt'], 'install': ['baz', 'quux']}}} {'virtualenv': {'a': {}, 'b': {'requirements': ['requirements.txt'], 'install': ['foo', 'bar', 'bla']}, 'c': {'link': ['bar', 'baz'], 'install': ['foo', '$HOME', '~/a']}}} {'virtualenv': {'magicExplodingVirtualenv': {}, 'a': {}, 'b': {}, 'c': {}}} There's a ticket (avakar/pytoml#6 <avakar/pytoml#6>) for pytoml to get object_pairs_hook like json. Maybe it's time for me to head over there. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#41>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUIXoixjtPSKsrtYi4wmYpsd7CbikHBks5t5eukgaJpZM4UaDfa> .

[altendky]

The linked test that's failing. It presumes that the magic one failing should keep 'c' from being created while still allowing 'a' and 'b' to be created. At least that was my cursory analysis of the assertion.

https://github.com/Julian/mkenv/blob/py3/mkenv/tests/test_converge.py#L134

[Julian]

Ahh got it, test bug. Yeah makes sense.

…

On Mon, Jun 4, 2018, 22:42 Kyle Altendorf ***@***.***> wrote: The linked test that's failing. It presumes that the magic one failing should keep 'c' from being created while still allowing 'a' and 'b' to be created. At least that was my cursory analysis of the assertion. https://github.com/Julian/mkenv/blob/py3/mkenv/tests/test_converge.py#L134 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#41 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUIXgqT6NzCOoC5LRzA1EWKBixrp9TWks5t5e_4gaJpZM4UaDfa> .

[altendky]

It seemed like that was a significant point of the test. Should it be truncated to only assert the exception and not the existence or lack thereof of the other environments? It seems that removing immediately_ from the test name would be proper in that case.

[Julian]

So the point of the test is to handle the case where an exception is handled in an environment before some other environments are to-be-installed.

(And means to say "it short circuits with that option after the exception")

So, the way it's written is wrong because it depends on hash order (which by the way is in-order on PyPy too which is why I didn't notice), but we need some other way of exercising that same scenario.

Probably by just forcing the returned dict into a particular order (by say passing in a modified toml.loads)

[altendky]

Other than just hacking in a tweak for the test...

1. Wait for pytoml to get object_pairs_hook (Naive addition of object_pairs_hook avakar/pytoml#39)
2. Add a CLI option to converge to natsort the loaded envs. Though I'm not sure what the use case would be for that other than this.
3. Always natsort the loaded envs. It's not obvious that this is a desired thing.

Or? I'm fine with # 1.

I guess now that I went and pulled a py3 branch, there wasn't really that much left to fix on py3 (at least as per reported by the tests). I should start actually using mkenv I suppose and figure out what py3 tests are missing. That and check it out on Windows.

[altendky]

Fixed in 338cbf7 by updating to latest pytoml with the object_pairs_hooks.