Thread (8 messages) 8 messages, 4 authors, 2024-04-02

Re: [RFC][GSoC] Proposal v2: Move more tests to the unit testing framework

From: Karthik Nayak <hidden>
Date: 2024-03-29 17:51:37

Chandra Pratap [off-list ref] writes:

Hello,
-----Contributions-----
Nit: Not sure how these contributions are sorted, but it would be nice
to group the git-for-windows patches together.
In GSoC
-------

-----Plan-----

Tests for Git are defined in the t/ directory and use the combination of
a helper file (written in C) and a shell script that invokes the said
helper file. I will use my work from the patch ‘tests: Move
t0009-prio-queue.sh to the unit testing framework’ to explain the steps
involved in the porting of such tests:

• Search for a suitable test to port:

As Christian Couder mentioned in this mail (link: https://public-inbox.org/git/CAP8UFD22EpdBU8HJqFM+=75EBABOTf5a0q+KsbzLK+XTEGSkPw@mail.gmail.com/),
there exists a subset of t/helper worth porting and we need some sort of
classification to discern these.

All helper files contain a cmd__foo() function which acts as the entry
point for that helper tool. For example, the helper/test-prio-queue.c
file contained cmd__prio_queue() which served as the entry point for
that file. The binary for the helper file is then mapped to a different
name by helper/test-tool.c which is used by the ‘*.sh’ files to perform
the tests. This name can be discovered by searching for the helper
file’s entry point in test-tool.c. Continuing the prior example,
“prio-queue” was the name for the helper/test-prio-queue.c binary and
t0009-prio-queue.sh invoked it like “prio-queue 1 2 get 3 dump”.
It is really nice to provide an example, but you're referring to files
which used to exist here, which can be a bit confusing. Would be nice to
talk about a new helper file you plan to port and what is the general
flow you'd follow there.
•  Create a new C test file in t/unit-tests:

After finding a test appropriate for the migration efforts, we create a
new ‘*.c’ file in t/unit-tests.  The test file must be named appropriately
to reflect the nature of the  tests it is supposed to perform.
Most of the times, replacing ‘tXXXX’  with ‘t-‘ and ‘*.sh’ with ‘.c’ in
the name of the test script suffices. E.g. t/t0009-prio-queue.sh turns
to t/unit-tests/t-prio-queue.c.
The new C file must #include “test-lib.h” (to be able to use the unit
testing framework) and other necessary headers files.

•  Move the code from the helper file:

Since the helper files are written in C, this step is mostly a
‘copy-paste then rename’ job. Changes similar to the following also need
to be made in the Makefile:
-    TEST_BUILTINS_OBJS += test-prio-queue.o
+    UNIT_TEST_PROGRAMS += t-prio-queue
This has a good potential for refactoring these files as they're added
to the new unit testing library.
•  Translate the shell script:

The trickiest part of the plan, since  different test scripts perform
various functions and a direct translation of the scripts to C is not
always optimal. Continuing the prior example, t0009-prio-queue.sh used a
single pattern for testing, write expected output to a temporary file
(named ‘expect’) -> feed input to the ‘prio-queue’ helper tool -> dump its
output to another temporary file (named ‘actual’) -> compare the two files
(‘actual’ vs ‘expect’).

In the first iteration of my prio-queue patch, I worked out a
straightforward translation of this pattern in C. I stored the input in
a string buffer, passed that buffer to the test function, stored its
output in another buffer and then called memcmp() on these two buffers.
While this did prove to be a working copy, this work was found to be inadequate
but why? It is important to state why this is not the best approach.
The test scripts similarly perform other functions like checking for
prerequisites, creating commits, initializing repositories, changing or
creating directories and so forth, and custom logic is required in most
of the cases of translating these, as seen above.

•  Run the resulting test, correct any errors:

It is rare for any migrated test to work correctly on the first run.
This step involves resolving any compile/runtime errors arising from the
test and making sure that at the very minimum, all the test-cases of the
original test are replicated in the new work. Improvements upon the original
can also be made, for example, the original t0009-prio-queue.sh did not
exercise the reverse stack functionality of prio-queue, which I improved
upon in unit-tests/t-prio-queue.

•  Send the resulting patch to the mailing list, respond to the feedback:

This step involves writing a meaningful commit message explaining each patch
in the series. From my experience contributing to the Git project, I find it
to be rare for any patch series to be accepted in the very first iteration.
Feedback from the community is vital for the refinement of any patch and
must be addressed by performing the suggested changes and sending the work
back to the mailing list. This must be repeated until the work is merged
into ‘seen’, ‘next’ and further down, ‘master’.

Timeline
--------

I’m confident that I can start the project as early as the start of the
Community Bonding Period (May 1 - 26). This is because I have read
the related documentation and already have some experience with the idea.
I believe I’ll be ready to get up to speed to work on the project by then.
The exact time arrangement for each test is variable and hard to determine,
but judging from the fact that it took me 3-4 days to complete the first
iteration of the t-prio-queue work, here is a proposed migration schedule:
I think the Community Bonding Period could also be utilized to go over
the patches submitted to date to move tests to the unit test framework
and draw learning's from them.
The first few steps of the plan are easy enough to knock out in a day,
the time required to port the tests depends mostly upon the work
required in translating the shell script. As mentioned previously, it
took me 3-4 days to complete the first iteration of the test-prio-queue
migration patch and that was a short test with only about 50 or so lines
of shell scripting and all the test cases following a single pattern.
Considering all this, I believe it should be possible, on average, to
migrate a new test in 4-7 days.
From there, it’s a matter of polishing the patch until integration with
‘master’ by addressing the feedback on the mailing list which can
deceptively take longer than expected. For instance, I had to continue
refining my t-prio-queue patch for around 2 weeks after the first
iteration to get it merged to ‘next’.
This is an important point, while you can probably have tests being
converted fast, it would be nicer to reduce the turn around time overall
and this could mean you spend more time at the start.

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help