mitre / caldera

Automated Adversary Emulation Platform
https://caldera.mitre.org
Apache License 2.0
5.54k stars 1.05k forks source link

Fix - Broken tests #3013

Open jbaptperez opened 2 months ago

jbaptperez commented 2 months ago

Description

Fixes:

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

All tests:

# Creating and activating a virtualenv in a .venv directory.
python -m venv .venv
. .venv/bin/activate

# Removing conflicting dnspython in requirements-dev.txt...
sed -i '/dnspython==2.1.0/d' requirements-dev.txt # Linux
sed -i '' '/dnspython==2.1.0/d' requirements-dev.txt # macOS
pip install -r requirements.txt -r requirements-dev.txt

# Installing node files (the required `plugins/magma/dist/index.html` file is to be generated)...
npm --prefix plugins/magma install
npm --prefix plugins/magma run build

# Executing all tests...
pytest --asyncio-mode=auto

# Applying linters...
pre-commit run --all-files --show-diff-on-failure

Checklist:

jbaptperez commented 2 months ago

I could fix all test failures locally (macOS), but it still remains tons of warnings (see CI/CD). I also fixed 2 CI/CD failures:

Now, the CI/CD still does not succeed: It looks like the SONAR_TOKEN environment variable is not set in GitHub. I think we are really close to completely restore testing capabilities.

What do you think @elegantmoose and @clenk? Any help?

jbaptperez commented 2 months ago

I tried to understand the remaining event loop warnings that appear when running tests, the ones that produce Task was destroyed but it is pending! or RuntimeError: Event loop is closed.

I focused on a single test that cause such warnings. Below is the command to produce the issue:

pytest --asyncio-mode=auto --tb=short tests/services/test_app_svc.py::TestAppService::test_mark_agent_as_untrusted_running_operation

After investigating, I figured out that the "destroyed" asyncio tasks come from the application itself. The application starts when the test method retrieves its fixtures.

That means when the test method start, It is already "too late" as the event loop has already a set of pending tasks. It also means that even with a completely empty body, the test method would produce the same warnings.

@bleepbop and @christophert, you both have worked on the tests. Do you think there is a proper way to fix that? I mean, something like starting a real "test-only light" application that does not start like in a normal one (without pending tasks).

I could reduce (but not completely suppress) the amount of warning by just applying this function at the beginning or the end of the test (whatever):

async def cancel_pending_tasks():
    loop = asyncio.get_running_loop()
    current_task = asyncio.current_task(loop)
    pending_tasks = [task for task in asyncio.all_tasks(loop) if task is not current_task and not task.done()]
    for task in pending_tasks:
        task.cancel()

However, this is not a clean way to fix that specific issue. Therefore, if you both don't have a solution to propose for now, I prefer stopping my investigations and let this pull request be merged (when the Sonar environment variable issue is solved, see above).

L015H4CK commented 2 months ago

Thanks for the great work!

I was working on some new features myself and ran into a lot of failing tests - wondering where they came from. Then I noticed they fail on the main branch as well. I just pulled your branch and the tests are fixed.

In my opinion, adding --asyncio-mode=auto to the pytest call is also important.

jbaptperez commented 1 month ago

While rebasing the work on the new master, I faced a regression introduced in commit 3ad8849 that broke some tests.

elegantmoose commented 1 month ago

@jbaptperez back on this, sorry for delay