(#2304) Make list --exact --all-versions to filter out prereleases by default #2308

vexx32 · 2021-08-02T13:35:25Z

Due to a prior fix in the logic for list -e -a, all packages with the target ID were being returned, regardless of whether the user uses the --pre flag or not. Since the same search without --exact does not return prerelease versions unless the --pre option is also passed, the behaviour for list -e -a should match this.

This fix modifies the search logic used when listing all package versions with --exact to also check for and respect the --pre flag from the configuration context, much like other list commands use the setting in this file in other places.

Changes

(breaking) choco list $x --all --exact no longer lists prerelease packages by default
choco list $x --all --exact --pre lists everything, prereleases included

Fixes #2304

Manual Testing / Verification

You can either build choco from this branch or modify the configuration in VS to start choco in debug mode with specific arguments. You will need to ensure you have access to the community repository in your configuration (and ideally nothing else, for the purposes of this particular test).

Run choco with arguments list python3 --exact --all
Verify all packages returned are stable (non-prerelease) versions
Run choco with arguments list python3 --exact --all --pre
Verify:
- The second command returns many more packages than the first.
- The second command includes the results from the first command + prerelease versions of the same package.
Run choco with arguments list python3 --exact
The latest stable version of python3 should be returned.
Run choco with arguments list python3 --exact --pre
The latest pre-release or stable version of python3 should be returned.
Confirm results by checking the python3 package page on the Community Repository and the versions currently available.

CLAassistant · 2021-08-02T13:35:30Z

All committers have signed the CLA.

vexx32 · 2021-08-02T14:50:14Z

Hrm, seems like the underlying Nuget lib doesn't like the resulting OData query here. Marking this as WIP until I have had a better look.

vexx32 · 2021-08-02T15:41:24Z

I have a solution for this, but leaving in WIP for now as we have some decisions to make. Before the changes that affected this command went in, in 0.10.13 the behaviour was:

choco list --exact --all lists ALL packages, prereleases included.
choco list --all lists no prereleases at all, but also misses many of the packages that --exact finds.

In other words, the problem this addresses was already in 0.10.13 before we made the recent breaks and fixes to the command logic. Will be following up internally to confirm this is the right avenue to go for fixing this issue, and will need to verify whether the issue with --all without --exact has another ticket already assigned, or we need to file another.

I'll push some additional changes shortly that will have this functional, but I expect we'll also want to add some tests around the expected behaviour here before we call this sorted, once a decision is made internally.

vexx32 · 2021-08-10T21:58:23Z

To follow up on my previous comment:

After following up with the team internally, the determination was that:
- choco list $x -e -a should list non-prerelease versions only
- choco list $a -e -a --pre should be respected and allow listing prerelease versions

We acknowledge this is a departure from established behaviour for this specific case (it was the same even back to 0.10.13, and likely earlier). This will make the choco list --exact --all cases behave similarly to other list command functions which don't retrieve prereleases without the --pre switch.

@gep13 I think this one is ready for review. I haven't gone too wild adding test cases, but I have added tests for the specific cases this should affect. Let me know what other cases we want to add here, if any.

pauby · 2021-08-11T09:08:14Z

Converting this to Draft until I can have a conversation about it.

pauby · 2021-08-12T15:03:51Z

Conversations had, removing it from Draft.

gep13

Minor nit-pick, but otherwise, LGTM!

gep13 · 2021-08-12T15:09:25Z

src/chocolatey.tests.integration/scenarios/ListScenarios.cs

@@ -496,8 +496,8 @@ public void should_contain_debugging_messages()
                MockLogger.contains_message("Start of List", LogLevel.Debug).ShouldBeTrue();
                MockLogger.contains_message("End of List", LogLevel.Debug).ShouldBeTrue();
            }
-        }        
-
+        }


Would prefer to see this whitespace change in a standalone commit.

gep13

LGTM!

Due to a prior fix in the logic for `list -e -a`, all packages with the target ID were being returned, regardless of whether the user uses the `--pre` flag or not. Since the same search without `--exact` does not return prerelease versions unless the `--pre` option is also passed, the behaviour for `list -e -a` should match this. This fix modifies the search logic used when listing all package versions with `--exact` to also check for and respect the `--pre` flag from the configuration context, much like other list commands use the setting in this file in other places. Also includes a minor refactor to avoid doing a double query here; now we only call the Search() function if we're actually going to use those results. The code paths for AllVersions completely discard that already, so this just removes the need to call the method unnecessarily.

gep13 · 2021-08-19T06:19:05Z

@vexx32 I have switched this PR to target the stable branch, and I will merge once the CI builds finish.

gep13 · 2021-08-19T06:28:12Z

@vexx32 thanks for getting this fixed up!

vexx32 requested review from AdmiringWorm and gep13 August 2, 2021 13:35

vexx32 changed the title ~~(#2304) Make list --exact --all-versions to filter out prereleases by default~~ WIP: (#2304) Make list --exact --all-versions to filter out prereleases by default Aug 2, 2021

vexx32 force-pushed the 2304/list-exact-all-filter-prereleases branch from 5be2fd4 to e7e1249 Compare August 2, 2021 15:43

vexx32 force-pushed the 2304/list-exact-all-filter-prereleases branch from e7e1249 to fcc7f84 Compare August 10, 2021 21:42

vexx32 changed the title ~~WIP: (#2304) Make list --exact --all-versions to filter out prereleases by default~~ (#2304) Make list --exact --all-versions to filter out prereleases by default Aug 10, 2021

pauby marked this pull request as draft August 11, 2021 09:07

pauby marked this pull request as ready for review August 12, 2021 15:04

gep13 requested changes Aug 12, 2021

View reviewed changes

vexx32 force-pushed the 2304/list-exact-all-filter-prereleases branch from fcc7f84 to f1704e8 Compare August 12, 2021 15:29

gep13 approved these changes Aug 19, 2021

View reviewed changes

vexx32 added 2 commits August 19, 2021 07:17

(maint) Remove unnecessary whitespace

50a21c4

gep13 force-pushed the 2304/list-exact-all-filter-prereleases branch from f1704e8 to 50a21c4 Compare August 19, 2021 06:18

gep13 changed the base branch from master to stable August 19, 2021 06:18

gep13 merged commit 85d522f into chocolatey:stable Aug 19, 2021

vexx32 mentioned this pull request Aug 19, 2021

choco list -e -a returns pre-releases even when --pre is not passed #2304

Closed

vexx32 deleted the 2304/list-exact-all-filter-prereleases branch August 31, 2021 21:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(#2304) Make list --exact --all-versions to filter out prereleases by default #2308

(#2304) Make list --exact --all-versions to filter out prereleases by default #2308

vexx32 commented Aug 2, 2021 •

edited

Loading

CLAassistant commented Aug 2, 2021 •

edited

Loading

vexx32 commented Aug 2, 2021

vexx32 commented Aug 2, 2021 •

edited

Loading

vexx32 commented Aug 10, 2021 •

edited

Loading

pauby commented Aug 11, 2021 •

edited

Loading

pauby commented Aug 12, 2021

gep13 left a comment

gep13 Aug 12, 2021

vexx32 Aug 12, 2021

gep13 left a comment

gep13 commented Aug 19, 2021

gep13 commented Aug 19, 2021

(#2304) Make list --exact --all-versions to filter out prereleases by default #2308

(#2304) Make list --exact --all-versions to filter out prereleases by default #2308

Conversation

vexx32 commented Aug 2, 2021 • edited Loading

Changes

Manual Testing / Verification

CLAassistant commented Aug 2, 2021 • edited Loading

vexx32 commented Aug 2, 2021

vexx32 commented Aug 2, 2021 • edited Loading

vexx32 commented Aug 10, 2021 • edited Loading

pauby commented Aug 11, 2021 • edited Loading

pauby commented Aug 12, 2021

gep13 left a comment

Choose a reason for hiding this comment

gep13 Aug 12, 2021

Choose a reason for hiding this comment

vexx32 Aug 12, 2021

Choose a reason for hiding this comment

gep13 left a comment

Choose a reason for hiding this comment

gep13 commented Aug 19, 2021

gep13 commented Aug 19, 2021

vexx32 commented Aug 2, 2021 •

edited

Loading

CLAassistant commented Aug 2, 2021 •

edited

Loading

vexx32 commented Aug 2, 2021 •

edited

Loading

vexx32 commented Aug 10, 2021 •

edited

Loading

pauby commented Aug 11, 2021 •

edited

Loading