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

Adding test for extension group example #5

Open
wants to merge 28 commits into
base: stefano/test/python-tests
Choose a base branch
from

Conversation

scoders-tob
Copy link

@scoders-tob scoders-tob commented Nov 13, 2019

Thanks @kumarak @Smjert

Smjert and others added 28 commits November 6, 2019 13:24
This is have better separation between them and the python tests.
BUCK files have been updated accordingly.

Changes to the internal generateCopyFileTarget function were needed.
The function now supports a base folder to be set so that the regex or file path
is appended to that base path, without having it included in the destination.
It will also not use a library target anymore, but a custom one so
that's possible to set properties with custom names.
- Add the bdist_wheel package to have pip optimize packages.
- Update Python 2 packages to Python 3
- Use pexpect==3.3
Restored all the python tests under tools/tests.
Some tests have been modified so that they can be relocated
and don't expect too specific hardcoded paths.

Fixed the gen_api function in genapi.py missing the path to the spec
file when evaluating blacklisted tables.

Updated the wiki docs with the optionally needed packages to run the
python tests.

Updated azure-pipelines.yml to install the needed packages for the
python tests.

Add an init process so that reaping works in the CI Docker image
- Update to Python3.
- Scripts now accepts the test configs directory as an argument.
- Update findOsquerydBinary to support Ninja builds
- Fix test_windows_service Administrator detection
- Avoid a ResourceWarning on BufferedReader when killing the osquery
process
- Fixed a ResourceWarning due to a child killed but not yet reaped.
The timeout concept was confusing, because it wasn't really
waiting for that amount of time and then interrupting the
tentatives. It was just trying timeout/interval times
which by default, in the expectTrue case, was 800 times, which
is a lot and compounds to a lot if the function tested did some
tentatives too.

timeout is now called attemps, and the default values have been
changed to something a bit saner.
psutil caches the results, so when accessing a previously found process,
it might hand a dead process handle, so accessing it throws an
exception.
Handle the exception and ignore the process.
- attempt variable has to be increased each loop,
  otherwise we end in an infinite loop.

- Corrected example_extension.ext path.
# There is a brief race between register and registry broadcast
# That fast external client fight when querying tables.
# Other config/logger plugins have wrappers to retry/wait.
time.sleep(0.5)
Copy link
Collaborator

@Smjert Smjert Nov 13, 2019

Choose a reason for hiding this comment

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

If this is caused by the fact that an extension is not immediately loaded when osquery starts, you can ask osquery to wait until such extension is loaded with --extensions_require=<extension name>.
Or it will wait at least up to the default interval (which is 3 seconds).
In this way you could also check that no error message is printed; specifically:
An error occured during extension manager startup: Required extension not found or not loaded: <extension name you used in the option>

Copy link
Author

Choose a reason for hiding this comment

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

Cool. I just reused code from existing tests though :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah oups, didn't saw that.
Well ok, it's true that the test will still fail if the time it waits is not enough because the results will be empty, but you will have a less precise knowledge of why.
Anyway, it will be for another time then.

Copy link
Author

@scoders-tob scoders-tob Nov 13, 2019

Choose a reason for hiding this comment

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

I can fix both this and test_91_extensions_settings then. I mean now!

@mike-myers-tob
Copy link

Current status:

there's an issue in how extensions group are dealt with which needs a small re-architecting

mainly in how they are presented to the user (currently you cannot have multiple extension groups, so you cannot have a test that tests them together with a valid extension group in the same build)

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.

4 participants