Re: [PATCH v10 00/15] Upstreaming the Scalar command

[Derrick Stolee <stolee@gmail.com>]

On 12/8/2021 6:15 AM, Johannes Schindelin wrote:
> Hi Junio,
> 
> On Sun, 5 Dec 2021, Junio C Hamano wrote:
> 
>> Elijah Newren <newren@gmail.com> writes:
>>
>>> From your wording it sounds like the plan might not include moving
>>> these tests over.  Perhaps it doesn't make sense to move them all
>>> over, but since they've caught problems in both Scalar and core Git,
>>> it would be nice to see many of those tests come to Git as well as
>>> part of a future follow on series.

Moving the C# test suite over doesn't make a lot of sense. We also
are re-using the test suite from VFS for Git, which is probably overkill
here. Those tests were created due to issues that arose with the virtual
filesystem (paired with the GVFS protocol for finding missing objects)
and most of them probably don't test anything interesting in Scalar.

When we _do_ find something interesting in that suite, we port over the
test as a normal Git test so the regression is avoided in the future.

We work to test the -rc0 version of every release with our custom patches
in microsoft/git and then run them through the Scalar and VFS for Git
functional tests as a necessary step before releasing to our internal
users. Since we are doing that already, it is a better use of time to
port tests that actually matter when they come up rather than port the
entire test suite.

>> Yeah, we may be initially queuing this without tests for expediency,
>> but a production code cannot go forever without CI tests to ensure
>> continued code health.  People make changes in other parts of the
>> system Scalar may depend on and unknowingly break some assumption
>> Scalar makes on it.

I think it is important to keep in mind that the Scalar features that
are being submitted here are getting Git-style tests included. The only
thing that is missing right now is a firm link with Git's CI system,
which can be added quickly once things have calmed down in the build
system.

If we are interested in doing something more substantial that is
closer to the Scalar functional tests, then it is important to know
that those tests are running against a production server to clone
data and fetch it dynamically throughout. That is not exactly something
we have done in the Git test suite before.

In fact, I don't think Scalar introduces anything novel here: if we
want to add more coverage of running Git commands while in a
sparse-checkout _and_ partial clone _and_ have a lot of optional config
set, then we can do that independently of Scalar. 'scalar clone' just
sets up a repository in a state that an expert user could do themselves,
so should we spend a lot of effort creating that environment in our
test suite?

We have this already in some form:

1. t1091 and t1092 try to cover important sparse-checkout behavior.

2. t0410, t5616, and others try to cover important partial clone
   behavior.

3. Our GIT_TEST_* variables that are enabled in one of our CI runs
   test many of the advanced config options enabled by Scalar.

The thing that is missing is "all of these things at once" which
would be difficult to do across the test suite with our current test
design. I'm happy to provide the service of checking the Scalar
functional tests before each release as an expensive way to check
that combination of configuration without adding that cost to every
CI run and developer inner loop.
 
> The Scalar Functional Tests were designed with Azure Repos in mind, i.e.
> they specifically verify that the `gvfs-helper` (emulating Partial Clone
> using the predecessor of Partial Clone, the GVFS protocol) manages to
> access the repositories in the intended way.
> 
> I do not know off-hand how entangled the GVFS part is in the test suite,
> but from what I recall, every single test starts with cloning a test
> repository. From Azure Repos. Using the `gvfs-helper`.

There is a single test that checks that 'scalar clone' against github.com
works appropriately [1]. We don't duplicate all of the other tests in
this environment.

[1] https://github.com/microsoft/scalar/blob/68b6e70d77f1c7c13be9f35848a042604f3fb2f1/Scalar.FunctionalTests/Tests/MultiEnlistmentTests/ScalarCloneFromGithub.cs

> Which means that the `gvfs-helper` would need to be upstreamed and be
> maintained in the git.git repository proper.
> 
> Previously I was under the impression that that might be met with grumpy
> rejection.
> 
> I do realize, though, that clarity of intention has been missing from this
> mail thread all around, so let me ask point blank: Junio, do you want me
> to include upstreaming `gvfs-helper` in the overall Scalar plan?

I, for one, don't think that has much value for the core Git project.

Thanks,
-Stolee