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

Replaces sleep based waiting for log entry lookups #167

Merged
merged 2 commits into from
Dec 10, 2022

Conversation

sr-gi
Copy link
Member

@sr-gi sr-gi commented Dec 9, 2022

Replaces time.sleep in the watchtower-plugin tests to wait for certain things to happen on the backend by wait_for_logs, which should be less error-prone.

Also does some reformating and removes unnecessary imports.

Fixes #163 #156 #90

@sr-gi sr-gi linked an issue Dec 9, 2022 that may be closed by this pull request
@sr-gi
Copy link
Member Author

sr-gi commented Dec 9, 2022

@booklearner you may be interested in this (b/c of #90)

@sr-gi
Copy link
Member Author

sr-gi commented Dec 9, 2022

I wanted to also address #156 here but I'm still unable to make it work consistently :(

@sr-gi sr-gi force-pushed the e2e-test-no-sleep branch from b18696b to 484cde3 Compare December 9, 2022 17:23
@booklearner
Copy link
Contributor

Copy link
Contributor

@booklearner booklearner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM.

@sr-gi sr-gi added tests Issues related with test functionality Seeking Code Review review me pls cln-plugin Stuff related to watchtower-plugin labels Dec 9, 2022
watchtower-plugin/tests/test.py Outdated Show resolved Hide resolved
watchtower-plugin/tests/test.py Show resolved Hide resolved
watchtower-plugin/tests/test.py Show resolved Hide resolved
watchtower-plugin/tests/test.py Show resolved Hide resolved
@sr-gi sr-gi force-pushed the e2e-test-no-sleep branch 2 times, most recently from a80d25f to fddd477 Compare December 9, 2022 18:24
@sr-gi
Copy link
Member Author

sr-gi commented Dec 9, 2022

I've improved the regexs to look for to be a bit less broad in fddd477

Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Replaces time.sleep in the `watchtower-plugin` tests to wait for certain things to happen
on the backend by wait_for_logs, which should be less error prone

Also does some reformating and removes unnecessary imports
@sr-gi sr-gi force-pushed the e2e-test-no-sleep branch from fddd477 to 5177446 Compare December 9, 2022 22:23
`TeosD` outputDir was being set to the same directory use by the test suite.
That cause it to be logging in the same log file, potentially colluding for
read/write operations. Setting the outputDir to it's own location fixed the issue.
@sr-gi sr-gi linked an issue Dec 9, 2022 that may be closed by this pull request
@sr-gi
Copy link
Member Author

sr-gi commented Dec 9, 2022

I ended up managing to fix #156! 🎉

Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sr-gi sr-gi merged commit dc02abd into talaia-labs:master Dec 10, 2022
@sr-gi sr-gi removed the Seeking Code Review review me pls label Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cln-plugin Stuff related to watchtower-plugin tests Issues related with test functionality
Projects
None yet
3 participants