google / gcp_scanner

A comprehensive scanner for Google Cloud
Apache License 2.0
305 stars 95 forks source link

⚡️ feat: add performance INFO in logs #185

Closed peb-peb closed 1 year ago

peb-peb commented 1 year ago

Description

NOTE: related docs for above topic

Changes Made

Checklist

Additional Notes

The goal is to improve the "Average Elapsed time since the program started" time significantly.

peb-peb commented 1 year ago

I'll implement it.

peb-peb commented 1 year ago

image

Currently, added the --time flag for "performance testing".

peb-peb commented 1 year ago

image

added tests for the --time params. (using pytest fixtures)

Also, should I add these tests for all parameters?

mshudrak commented 1 year ago

Yes, it would be great to have them for all parameters in the future.

mshudrak commented 1 year ago

oops, forgot to take a look but pylint is complaining about several places. Please fix.

peb-peb commented 1 year ago

fixed the pylint errors and the changes requested.

mshudrak commented 1 year ago

Hmm, the new test is failing at the moment

==================================== ERRORS ====================================
_________________________ ERROR at setup of test_name __________________________

request = <SubRequest 'perf_time' for <Function test_name>>

    @pytest.fixture()
    def perf_time(request):
>     time_value = request.config.option.time
E     AttributeError: 'Namespace' object has no attribute 'time'

src/gcp_scanner/conftest.py:11: AttributeError
=========================== short test summary info ============================
ERROR src/gcp_scanner/test_params.py::test_name - AttributeError: 'Namespace' object has no attribute 'time'
=================== 53 passed, 1 error in 102.36s (0:01:42) ====================
Error: Process completed with exit code 1.
peb-peb commented 1 year ago

=========================== short test summary info ============================ ERROR src/gcp_scanner/test_params.py::test_name - AttributeError: 'Namespace' object has no attribute 'time'

It is failing because it is unable to find the time attribute/parameter (which is being added in this PR). It would PASS - if the time parameter is passed or SKIP - if the time parameter is absent in tests, once this PR is merged.

mshudrak commented 1 year ago

I reverted this one since once it was merged it failed the tests. Seems like the issue somewhere else. PTAL.