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

Fix linter errors in test/e2e-coinjoin-test.py #1229

Merged

Conversation

kristapsk
Copy link
Member

test/e2e-coinjoin-test.py:20:1: F401 'random' imported but unused
test/e2e-coinjoin-test.py:28:1: F401 'jmclient.wallet_utils.wallet_gettimelockaddress' imported but unused

@AdamISZ
Copy link
Member

AdamISZ commented Apr 9, 2022

Thanks, I usually remember to do flake8 locally but I keep forgetting to add test/ to it. I can't remember, but I'm guessing we didn't add it to CI because something in there fails?

(Btw reminds me: I'll just remove test/test_full_coinjoin.py now we have this new e2e* test which does a similar job, though I'm not sure whether to add it to the test suite).

@kristapsk
Copy link
Member Author

Thanks, I usually remember to do flake8 locally but I keep forgetting to add test/ to it.

./test/lint/lint-python.sh :)

I can't remember, but I'm guessing we didn't add it to CI because something in there fails?

Not sure either, could try add in #1218.

@kristapsk
Copy link
Member Author

though I'm not sure whether to add it to the test suite

Why not?

@AdamISZ
Copy link
Member

AdamISZ commented Apr 9, 2022

though I'm not sure whether to add it to the test suite

Why not?

Yeah I couldn't remember offhand; investigating now. Hopefully we can.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 9, 2022

Going to merge this now so I can base #1230 off it, where I will also address the point above (adding the e2e test into it).

@AdamISZ AdamISZ merged commit 68c8d4a into JoinMarket-Org:master Apr 9, 2022
@kristapsk kristapsk deleted the e2e-coinjoin-test-linter-fix branch April 9, 2022 17:59
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