apacheconfig icon indicating copy to clipboard operation
apacheconfig copied to clipboard

Test writable loader with apache config files

Open sydneyli opened this issue 6 years ago • 2 comments

Depends on #73. Check out the last two commits for the full diff.

In Certbot, we have a lot of sample config files that we test against. Here's a bunch from issues in the past: https://github.com/certbot/certbot/tree/5b45d0742ad51bb9ad93dd61977b5b2d4bcc4319/certbot-apache/certbot_apache/tests/apache-conf-files

And we also test against the default system apache configurations on different OSes: https://github.com/certbot/certbot/tree/5b45d0742ad51bb9ad93dd61977b5b2d4bcc4319/certbot-apache/certbot_apache/tests/testdata

I started testing apacheconfig against these files-- it might be good to check in a bunch of these files, to prevent future regressions. Let me know what you think.

This is a draft pull request for now! Going to add some more test configs.

EDIT: Woah! this is a huge diff. Most of it is just really long config files. The only piece of code added in this PR should be a new integration test at tests/integration/test_apache_samples.py.

sydneyli avatar Oct 04 '19 02:10 sydneyli

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@27845ec). Click here to learn what that means. The diff coverage is 31.2%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #74   +/-   ##
=========================================
  Coverage          ?   81.55%           
=========================================
  Files             ?        7           
  Lines             ?      759           
  Branches          ?        0           
=========================================
  Hits              ?      619           
  Misses            ?      140           
  Partials          ?        0
Impacted Files Coverage Δ
apacheconfig/__init__.py 100% <100%> (ø)
apacheconfig/wloader.py 29.5% <29.5%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 27845ec...7368213. Read the comment docs.

codecov-io avatar Oct 04 '19 02:10 codecov-io

If you anticipate maintaining a large collection of sample files, may be it would make sense to push them all into a separate package just for testing (e.g. apacheconfig-tests)...?

Yeah, we chatted about this briefly as well (breaking out our test files from Certbot). For the time being, I think this is alright, but if we find ourselves updating this set of files frequently, it would definitely make more sense to pull it out into an independent testing package that both Certbot and apacheconfig can depend on.

sydneyli avatar Oct 31 '19 19:10 sydneyli