clear description cache if updated w/o EVAL_ALL

[rrotter]

When updating description cache, if eval_all not set, clear cache rather than leaving it out of date.

This fixes an issue where, if a user sets --eval-all on the command line to run description searches, but HOMEBREW_EVAL_ALL isn't set in the environment the cache is never updated.

• [x] Have you followed the guidelines in our Contributing document?
• [x] Have you checked to ensure there aren't other open Pull Requests for the same change?
• [x] Have you added an explanation of what your changes do and why you'd like us to include them?
• [x] Have you written new tests for your changes? Here's an example.
• [x] Have you successfully run brew style with your changes locally?
• [x] Have you successfully run brew typecheck with your changes locally?
• [x] Have you successfully run brew tests with your changes locally?

---

[rrotter]

Workflow question: when I've got a bug and a solution to it (as is the case here), should I submit an issue before sending a PR?

[apainintheneck]

Workflow question: when I've got a bug and a solution to it (as is the case here), should I submit an issue before sending a PR?

If you plan on opening a PR for a bug right away, it's usually better to just do that directly and not open an accompanying issue. If you're not planning on working on something immediately or have a feature request, issues are the better choice.

[rrotter]

I'm not sure what else to add in terms of testing. I'm happy to take another pass at it if either of you have any suggestions. Otherwise, I'm fine with this merging as-is if you are.

[apainintheneck]

It might be possible to add some more integration style tests to the Library/Homebrew/test/cmd/update-report_spec.rb and the Library/Homebrew/test/tap_spec.rb that check if the cache files exist when HOMEBREW_EVAL_ALL is set and when it's not. Those tests would be more useful but also harder to get working. I'm with fine with the change as is since it's pretty small.

[MikeMcQuaid]

It might be possible to add some more integration style tests to the Library/Homebrew/test/cmd/update-report_spec.rb and the Library/Homebrew/test/tap_spec.rb that check if the cache files exist when HOMEBREW_EVAL_ALL is set and when it's not. Those tests would be more useful but also harder to get working. I'm with fine with the change as is since it's pretty small.

@apainintheneck We cannot keep adding more command integration tests to cover these edge cases. They are far too slow. More tests for test/tap_spec.rb would be fine.

We used to have a "rule" of only having one per command that has gone out the window recently. We should really be going back to that for all but the most essential/configurable commands e.g. install/upgrade.

[apainintheneck]

@apainintheneck We cannot keep adding more command integration tests to cover these edge cases. They are far too slow. More tests for test/tap_spec.rb would be fine.

We used to have a "rule" of only having one per command that has gone out the window recently. We should really be going back to that for all but the most essential/configurable commands e.g. install/upgrade.

@MikeMcQuaid When I said integration style test, I basically meant a test that is in between a unit test that tests only a single method and an end-to-end test that tests the entire system.

It might be possible to reuse some of the Homebrew::Cmd::UpdateReport::Reporter test logic since it already creates a ReporterHub which is what gets passed to the DescriptionCacheStore#update_from_report! method. Honestly though it's likely not worth the trouble.

[MikeMcQuaid]

@apainintheneck Ok, thanks for clarifying. In the Homebrew context: I always assume :integration_test.

[apainintheneck]

I'm merging this in as is since it's a small change that I don't expect to cause problems. Also, tests could always be added in another PR too.

Thanks for fixing this @rrotter!