choco icon indicating copy to clipboard operation
choco copied to clipboard

(#1332) Add tests for disabling file logging of archive extractions

Open AdmiringWorm opened this issue 2 years ago • 1 comments

Description Of Changes

This pull request adds two new E2E tests that allows us to verify the behavior of when a maintainer disables file logging when extracting archives. One of the new tests will only run through our internal test kitchen instance.

Motivation and Context

This is done to try preventing regressions from happening on new functionality that gets implemented.

Testing

  1. Ran both of the new tests through test kitchen.

Change Types Made

  • [ ] Bug fix (non-breaking change)
  • [ ] Feature / Enhancement (non-breaking change)
  • [ ] Breaking change (fix or feature that could cause existing functionality to change)
  • [ ] PowerShell code changes.
  • [x] Tests

Related Issue

#1332
https://app.clickup.com/t/20540031/ENGTASKS-1480

Change Checklist

  • [ ] Requires a change to the documentation
  • [ ] Documentation has been updated
  • [x] Tests to cover my changes, have been added
  • [x] All new and existing tests passed.
  • [ ] PowerShell v2 compatibility checked.

AdmiringWorm avatar Sep 21 '22 16:09 AdmiringWorm

Task linked: ENGTASKS-1480 Chocolatey CLI

choco-bot avatar Sep 21 '22 16:09 choco-bot

Just the question around enabling the local repository. Perhaps Test Kitchen should just keep it enabled by default?

Enabling the local repository should honestly just be avoided. It was mainly just enabled while I was working on the tests before I made the internal PR to create the needed packages.

If we enable it by default, it may have adverse side affects with other tests based on what we have available in the packages directory and could invalidate the tests.

AdmiringWorm avatar Sep 22 '22 12:09 AdmiringWorm

Coverage Status

Coverage remained the same at 27.592% when pulling 13968e9bf2c057ecd033ae5cb1de16e61d91d9eb on AdmiringWorm:logging-tests into aa184b33143c5af4909a39b3d5abab48116af8a3 on chocolatey:develop.

coveralls avatar Sep 22 '22 13:09 coveralls

What about changing the local package id to be include local, and then we can reduce the two context blocks to one with a ForEach?

Discussed this in Slack, it's on purpose that they're separate because external is an Internal tagged test because it requires infrastructure that won't be there without access to internal infrastructure.

corbob avatar Sep 22 '22 15:09 corbob

Thanks for getting these added @AdmiringWorm!

corbob avatar Sep 22 '22 15:09 corbob