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

tests: Keep proc instance for test_base and test_osqueryd #6335

Merged
merged 1 commit into from
Mar 28, 2020

Conversation

theopolis
Copy link
Member

This is required for #6334.

I am unsure why the "runner"'s proc member was removed at the end of a successful run()?

But without this instance we cannot inspect the daemon's pid. In the follow up PR we confirm the daemon exits correctly.

@theopolis theopolis added the test label Mar 28, 2020
@Smjert
Copy link
Member

Smjert commented Mar 28, 2020

Ah good catch!
The problem was that the base framework doesn't always correctly cleanup resources when an operation has finished and this either keeps processes running or make the test fail in other parts.

I will eventually get to it again, but that framework needs a refactor.

EDIT:
Checking again, the intention was the above and to save the pid for later inspection, although what I missed at the time is that pid is a property, not a simple class member variable, that has to access the proc member.
I think it's fine to bring it back as it was, although as I was saying this needs fixing eventually.

@theopolis theopolis merged commit bb861fb into osquery:master Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants