search

18F / domain-scan

A lightweight pipeline, locally or in Lambda, for scanning things like HTTPS, third party service use, and web accessibility.
Other
370 stars 137 forks source link

Abandon OO experiment, refactor scan code path, add tests #241

Closed tadhg-ohiggins closed 6 years ago

tadhg-ohiggins commented 6 years ago

We should hand things off, Not seek to carry them within. Separate each task.

Remove OO execution paths, refactor and add tests for various parts of scan execution flow, refactor and add tests for analytics scanner, add support for explicit argument handling for each scanner as a separate function.

After struggling with making an OO-based approach work and running into too many issues, and considering that handing functions off to Lambda requires them to have all context available to them anyway, I realized that the only benefit we were getting from the OO approach was the elegance of abstract methods enforcing what methods were mandatory for scanners. Instead of that elegance, we now have a check when the scanner module is loaded to verify their presence—but less OO overhead.

I gave this the do_not_merge_yet label both because there are quite a few changes to the main code path that might break corner cases I've missed, and to leave time for arguments for returning to the OO approach.

tadhg-ohiggins commented 6 years ago

@konklone Was there anything else in this you wanted to change before approving it? Have you found any more invocations that break using this branch?