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

Add Dockerfile with test requirements #125

Closed
wants to merge 3 commits into from

Conversation

3nprob
Copy link

@3nprob 3nprob commented Apr 25, 2022

Depends on lndmanage:local from bitromortac/lnregtest#7

@3nprob
Copy link
Author

3nprob commented Apr 25, 2022

I am getting one failed test on current master - is that the case for you as well when you run in your environment @bitromortac ?

======================================================================
ERROR: test_circle (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_circle
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/unittest/loader.py", line 436, in _find_test_path
    module = self._get_module_from_name(name)
  File "/usr/local/lib/python3.10/unittest/loader.py", line 377, in _get_module_from_name
    __import__(name)
  File "/root/lndmanage/test/test_circle.py", line 9, in <module>
    from lndmanage.lib.listings import ListChannels
  File "/root/lndmanage/lndmanage/lib/listings.py", line 14, in <module>
    from lndmanage.lib.forwardings import (
  File "/root/lndmanage/lndmanage/lib/forwardings.py", line 9, in <module>
    from lndmanage.lib.node import LndNode
  File "/root/lndmanage/lndmanage/lib/node.py", line 21, in <module>
    from lndmanage.lib.network import Network
  File "/root/lndmanage/lndmanage/lib/network.py", line 11, in <module>
    from lndmanage.lib.rating import ChannelRater
  File "/root/lndmanage/lndmanage/lib/rating.py", line 4, in <module>
    from lndmanage import settings
  File "/root/lndmanage/lndmanage/settings.py", line 121, in <module>
    set_lndmanage_home_dir()
  File "/root/lndmanage/lndmanage/settings.py", line 72, in set_lndmanage_home_dir
    check_or_create_configuration(home_dir)
  File "/root/lndmanage/lndmanage/lib/configure.py", line 76, in check_or_create_configuration
    exit(0)
  File "/usr/local/lib/python3.10/_sitebuiltins.py", line 26, in __call__
    raise SystemExit(code)
SystemExit: 0


======================================================================
FAIL: test_liquidity_hints (test_pathfinding.TestGraph)
3
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/lndmanage/test/test_pathfinding.py", line 100, in test_liquidity_hints
    self.assertEqual(['A', 'D', 'E'], path)
AssertionError: Lists differ: ['A', 'D', 'E'] != ['A', 'B', 'E']

First differing element 1:
'D'
'B'

- ['A', 'D', 'E']
?        ^

+ ['A', 'B', 'E']
?        ^


----------------------------------------------------------------------
Ran 25 tests in 497.259s

FAILED (failures=1, errors=1)

@3nprob
Copy link
Author

3nprob commented Apr 25, 2022

ImportError: Failed to import test module: test_circle seems to be because the test runner seems to be because just importing lndmanage.settings` will trigger attempting to use the default homedir, before it can be overriden.

set_lndmanage_home_dir()

The test_liquidity_hints test failure seems to be something else

@3nprob
Copy link
Author

3nprob commented Apr 25, 2022

Added a kind of dirty fix reordering imports and setting default value for LNDMANAGE_HOME, which fixes the import error.

Remaining failure on run now:

======================================================================
FAIL: test_liquidity_hints (test_pathfinding.TestGraph)
3
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/lndmanage/test/test_pathfinding.py", line 102, in test_liquidity_hints
    self.assertEqual(['A', 'D', 'E'], path)
AssertionError: Lists differ: ['A', 'D', 'E'] != ['A', 'B', 'E']

First differing element 1:
'D'
'B'

- ['A', 'D', 'E']
?        ^

+ ['A', 'B', 'E']
?        ^


----------------------------------------------------------------------
Ran 27 tests in 630.117s

FAILED (failures=1)

@bitromortac
Copy link
Owner

Cool you decided to work on that. I know that one or two tests are flaky (didn't have yet time to go after it in detail), try to run them in isolation. Going to revive #84 soon, so tests could change a bit. Perhaps it would make sense to hold this back a bit.

@bitromortac
Copy link
Owner

bitromortac commented Apr 25, 2022

Ok, I pushed some raw code of how the new async tests could look like in #84. I described a bit what I'm trying to do there, feedback is welcome 🙂

@3nprob
Copy link
Author

3nprob commented Apr 26, 2022

try to run them in isolation

How do I indicate which tests to run individually? I'm new to this way of testing Python and got errors doing it my naive way

@3nprob
Copy link
Author

3nprob commented Apr 26, 2022

Cool you decided to work on that. I know that one or two tests are flaky (didn't have yet time to go after it in detail), try to run them in isolation. Going to revive #84 soon, so tests could change a bit. Perhaps it would make sense to hold this back a bit.

Got it. #84 still seems to have the issue with the test suite attempting to use/initiate a production config that I fixed in d60504d - would be nice if that can be resolved separately.

(I'd also argue that #84 is quite big already and touches many parts and functions - making the tests run reliably separately before that, and any test overhaul not directly tied to the changes in it, would be ideal, to have higher confidence that existing functionality does not break. This includes the making-tests-async-changes. In general it's good idea to split things like that up IMO. Left a separate comment on that there as well!)

@3nprob
Copy link
Author

3nprob commented May 1, 2022

Addressed the failing tests in #129 (should be independent of all currently open PRs)

@3nprob
Copy link
Author

3nprob commented May 5, 2022

Superseded by #128

@3nprob 3nprob closed this May 5, 2022
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