search

enthought / envisage

Envisage is a Python-based framework for building applications whose functionalities can be extended by adding "plug-ins".
http://docs.enthought.com/envisage/
Other
82 stars 26 forks source link

Fix failing cron job #468

Closed rahulporuri closed 2 years ago

mdickinson commented 2 years ago

@rahulporuri Do you happen to know which upstream change triggered the need for this one?

mdickinson commented 2 years ago

I'm a bit puzzled by the remaining failure. I get:

mdickinson@mirzakhani envisage % python -m unittest envisage.ui.action.tests.test_action_manager_builder.ActionManagerBuilderTestCase.test_duplicate_menu
F
======================================================================
FAIL: test_duplicate_menu (envisage.ui.action.tests.test_action_manager_builder.ActionManagerBuilderTestCase)
duplicate menu
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/mdickinson/Enthought/ETS/envisage/envisage/ui/action/tests/test_action_manager_builder.py", line 634, in test_duplicate_menu
    ["NewGroup", "ExitGroup", "ExtraGroup", "additions"], ids
AssertionError: Lists differ: ['NewGroup', 'ExitGroup', 'ExtraGroup', 'additions'] != ['NewGroup', 'ExtraGroup', 'ExitGroup', 'additions']

First differing element 1:
'ExitGroup'
'ExtraGroup'

- ['NewGroup', 'ExitGroup', 'ExtraGroup', 'additions']
?             -------------

+ ['NewGroup', 'ExtraGroup', 'ExitGroup', 'additions']
?                            +++++++++++++

----------------------------------------------------------------------

But as far as I can see, this test hasn't changed recently, so I'm not sure what caused it to start failing.

rahulporuri commented 2 years ago

@rahulporuri Do you happen to know which upstream change triggered the need for this one?

Nope, I didn't try tracking that down, I blindly started to try and fix things.

But as far as I can see, this test hasn't changed recently

But I did change the test in this PR - https://github.com/enthought/envisage/blob/92d827ef392a86807a491443f6975750e97c90cb/envisage/ui/action/tests/test_action_manager_builder.py#L584-L591 and https://github.com/enthought/envisage/blob/92d827ef392a86807a491443f6975750e97c90cb/envisage/ui/action/tests/test_action_manager_builder.py#L600-L606

mdickinson commented 2 years ago

@rahulporuri Okay, I think this is ready to merge, if the EDM builds pass. Does it look okay to you in its current form?

We have what looks like a brand new issue to deal with with the PyPI build.

rahulporuri commented 2 years ago

Does it look okay to you in its current form?

Yes it does. Thanks @mdickinson !

mdickinson commented 2 years ago

Merging. The PyPI failures are due to #469, and are independent of the changes in this PR. I'll tackle those next.

Thanks, @rahulporuri!