{"thread":{"id":"61153","subject":"[RFC][GSoC] Proposal: Move reftable and other tests to the unit testing framework","startedAt":"2024-03-19T17:11:40Z","lastAt":"2024-04-02T10:02:14Z","messageCount":8,"participants":["Chandra Pratap","Christian Couder","Patrick Steinhardt","Karthik Nayak"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"490936","messageId":"6f0a3c13-c8d9-4f89-8c62-9c031f0a064e@gmail.com","threadId":"61153","inReplyTo":null,"subject":"[RFC][GSoC] Proposal: Move reftable and other tests to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-03-19T17:11:26Z","receivedAt":"2024-03-19T17:11:40Z","isPatch":false,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"This is my project proposal for the Google Summer of Code 2024 program.\nThe document version of this proposal can be accessed through this link:\nhttps://shorturl.at/ijrTU\n\n---------<8----------<8----------<8----------<8----------<8----------<8\n\nPersonal Info\n-------------\n\nFull name: Chandra Pratap\nPreferred name: Chand\n\nE-mail: chandrapratap3519@gmail.com\nPhone: (+91)77618-24030\n\nEducation: SV National Institute of Technology, Surat, India\nYear: Sophomore (2nd)\nMajor: Mathematics\n\nGitHub: https://github.com/Chand-ra\n\nBefore GSoC\n-----------\n\n-----Synopsis-----\n\nA new unit testing framework was introduced to the Git mailing list last\nyear with the aim of simplifying testing and improving maintainability.\nThe idea was accepted and merged into master on 09/11/2023. This project\naims to extend that work by moving more tests from the current setup to\nthe new unit testing framework.\n\nThe SoC 2024 Ideas page (link: https://git.github.io/SoC-2019-Ideas/)\nmentions reftable unit tests migration as a separate project from the\ngeneral unit test migration project, however, I propose migrating other\ntests alongside the reftable unit tests as a part of this proposal.\nThe reasoning behind this is explained further down.\nThe difficulty for the project should be medium and it should take \nsomewhat between 175 to 350 hours.\n\n-----Contributions-----\n\n• apply.c: make git apply respect core.fileMode settings\n-> Status: merged into master\n-> link: https://public-inbox.org/git/20231226233218.472054-1-gitster@pobox.com/\n\n-> Description: When applying a patch that adds an executable file, git \napply ignores the core.fileMode setting (core.fileMode specifies whether\nthe executable bit on files in the working tree are to be honored or not)\nresulting in false warnings. Fix this by inferring the correct file mode \nfrom the existing index entry when core.filemode is set to false. Add a\ntest case that verifies the change and prevents future regression.\n\n-> Remarks: This was the first patch I worked on as a contributor to Git.\nServed me as an essential intro lesson to the community’s working flow and\ngeneral practices.\n\n• tests: Move t0009-prio-queue.sh to the unit testing framework\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com/\n\n-> Description: t/t0009-prio-queue.sh along with t/helper/test-prio-queue.c\nunit test Git's implementation of a priority queue. Migrate the test\nover to the new unit testing framework to simplify debugging and reduce\ntest run-time.\n\n-> Remarks: Perhaps the most relevant patch of all the ones mentioned\nhere, this patch helped me understand the expectations and workflow for\nthe work to be performed in this project.\n\n• write-or-die: make GIT_FLUSH a Boolean environment variable\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com/\n\n-> Description: Among Git's environment variable, the ones marked as\n\"Boolean\" accept values like Boolean configuration variables, i.e. \nvalues like 'yes', 'on', 'true' and positive numbers are taken as \"on\"\nand  values like 'no', 'off','false' are taken as \"off\". Make GIT_FLUSH \naccept more values besides '0' and '1' by turning it into a Boolean \nenvironment variable & update the related documentation.\n\n• sideband.c: remoye redundant NEEDSWORK tag\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1625.v4.git.1703750460527.gitgitgadget@gmail.com/\n\n-> Description: Replace a misleading NEEDSWORK tag in sideband.c that\nreads ‘replace int with size_t’ with another comment explaining why it\nis fine to use ‘int’ and the replacement isn’t necessary.\n\n• make tig callable from PowerShell/Command Prompt\n-> Status: merged into main\n-> link: https://github.com/git-for-windows/MINGW-packages/pull/104\n\n-> Description: Tig is a text mode interface for Git that ships with the\nstandard Git for Windows package but isn’t callable from PowerShell/\nCommand Prompt by default. Fix this by updating the relevant Makefiles\nand resource scripts.\n\n• fix broken link on Git for Windows’ GitHub wiki\n-> Status: merged\n-> link: https://github.com/git-for-windows/git/wiki/Home/_history\n\n-> Remarks: A simple fix for a broken link that I stumbled upon while\nbrowsing Git for Windows’ wiki looking for some help with the patch \nmentioned just before this one.\n\n• t4129: prevent loss of exit codes due to the use of pipes\n-> Status: merged into master\n-> link: https://lore.kernel.org/git/20220311132141.1817-1-shaoxuan.yuan02@gmail.com/\n\n-> Description: Piping the output of git commands like git-ls-files to\nanother command (grep in this case) in t4129 hides the exit code returned\nby these commands. Prevent this by storing the output of git-ls-files to\na temporary file and then \"grep-ping\" from that file. Replace grep with\ntest_grep as the latter is more verbose when it fails.\n\n• t9146: replace test -d/-f with appropriate test_path_is_* function\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1661.v3.git.1707933048210.gitgitgadget@gmail.com/\n\n-> Description: The helper functions test_path_is_* provide better debugging\ninformation than test -d/-e/-f.\nReplace tests -d/-e/-f with their respective ‘test_path_is_foo’ calls.\n\n• regex: update relevant files in compat/regex\n-> Status: WIP\n-> link: https://github.com/gitgitgadget/git/pull/1641\n\n-> Description: The RegEx code in compat/regex has been vendored from \ngawk and was last updated in 2010. This may lead to performance issues \nlike high CPU usage. Fix this by synchronizing the relevant files in \ncompat/regex with the latest version from GNULib and then replaying any\nchanges we made to gawk’s version on top of the new files.\n\n-> Remarks: When I started working on this patch, I thought it was an\neasy fix but the work turned out to be more involved than I anticipated.\nI had to seek help from the other community members, and we have made\nsome good progress, but there is still a lot of cleaning and changes that\nneed to be done. I haven’t found enough time to commit to this again,\nbut it’s surely something that I want to get done soon.\n\n• tests: Move t0032-reftable-unittest.sh to the unit testing framework\n-> Status: WIP\n-> link: https://github.com/gitgitgadget/git/pull/1698\n\n-> Description: t/t0032-reftable-unittest.sh along with t/helper/test-reftable.c\nunit test Git’s reftable framework. Migrate the test over to the new\nunit testing framework to simplify debugging and reduce test run-time.\n\n-> Remarks: An infant version as of now, I tinkered with this after \nseeing the project list on 'Git SoC 2024 Ideas' page to get an idea of\nthe kind of work that will be involved in this project.\n\n-----Related Work-----\n\nPrior works about the idea have been performed by other community members\nand previous interns which form a good guiding path for my own approach.\nSome previous example work:\n\ni) Port helper/test-ctype.c to unit-tests/t-ctype.c\n-> link: https://lore.kernel.org/git/20240112102743.1440-1-ach.lumap@gmail.com/\n\nii) Port test-sha256.c and test-sha1.c to unit-tests/t-hash.c\n-> link: https://lore.kernel.org/git/20240229054004.3807-2-ach.lumap@gmail.com/\n\niii) Port helper/test-date.c to unit-tests/t-date.c\n-> link: https://lore.kernel.org/git/20240205162506.1835-2-ach.lumap@gmail.com/\n\niv) Port test-strcmp-offset.c to unit-tests/t-strcmp-offset.c\n-> link: https://lore.kernel.org/git/20240310144819.4379-1-ach.lumap@gmail.com/\n\nv) Integrate a simple strbuf unit test with Git's Makefiles\n-> link: https://lore.kernel.org/git/20230517-unit-tests-v2-v2-4-21b5b60f4b32@google.com/\n\nvi) t0080: turn t-basic unit test into a helper\n-> link: https://lore.kernel.org/git/a9f67ed703c8314f0f0507ffb83b503717b846b3.1705443632.git.steadmon@google.com/\n\nIn GSoC\n-------\n\n-----Plan-----\n\n-> Reftable tests:\n\nThe reftable tests are different from other tests in the test directory\nbecause they perform unit testing with the help of a custom test framework\nrather than the usual ‘helper file + shell script’ combination.\nReftable tests do have a helper file and a shell script invoking the\nhelper file, but rather than performing the tests, this combination is\nused to invoke tests defined in the reftable directory.\n    The reftable directory consists of nine tests:\n\n•  basics test\n•  record test\n•  block test\n•  tree test\n•  pq test\n•  stack test\n•  merged test\n•  refname test\n•  read-write test\n\nEach of these tests is written in C using a custom reftable testing \nframework defined by reftable/test_framework (also written in C). The\nframework has four major features utilized in performing the tests:\n\n•  EXPECT_ERR(c): A function-like macro that takes as input an integer\n‘c’ (generally the return value of a function call), compares it against\n0 and spits an error message if equality doesn’t hold. The error message\nitself contains information about the file where this macro was used,\nthe line in this file where the macro was called and the error code ‘c’\ncausing the error.\n\n•  EXPECT_STREQ(a, b): A function-like macro that takes as input two \nstrings ‘a’ and ‘b’, compares them for equality via strcmp() and throws an\nerror if equality doesn’t hold. The error message thrown contains information\nregarding the file where this macro was invoked, the line in this\nfile where the macro was called and the mismatched strings ‘a’ and ‘b’.\n\n•  EXPECT(c): A function-like macro that takes as input an integer ‘c’\n(generally the result of a Boolean expression like a == b) and throws an\nerror message if c == 0. The error message is similar to EXPECT_ERR(c).\n\n•  RUN_TEST(f): A function-like macro that takes as input the name of a\nfunction ‘f’ (a test function that exercises a part of reftable’s code),\nprints to stdout the message ‘running f’ and then calls the function with f().\n\nOther than these, the framework consists of two additional functions,\nset_test_hash() and strbuf_add_void() which are used  exclusively in the\nstack tests and refname tests respectively.\n\nSince the reftable test framework is written in C like the unit testing\nframework, we can create a direct translation of the features mentioned\nabove using the existing tools in the unit testing framework with the\nfollowing plan:\n\n•  EXPECT_ERR(c): Can be replaced by check(!c) or check_int(c, “==”, 0).\n\n•  EXPECT_STREQ(a, b): Can be replaced by check_str(a, b). \n\n•  EXPECT(c): Can be replaced by check_int(), similar to EXPECT_ERR.\n   E.g. expect(a >= b) --> check_int(a, “>=”, b)\n\n•  RUN_TEST(f): Can be replaced by TEST(f(), “message explaining the test”).\n\nThe information contained in the diagnostic messages of these macros is\nreplicated in the unit testing framework by default. Any additional \ninformation can be displayed using the test_msg() functionality in the\nframework. The additional functions set_test_hash() and strbuf_add_void()\nmay be moved to the source files for stack and refname respectively.\n\nThe plan itself is basic and might need some modifications, but using\nthe above plan, I have already created a working albeit primitive copy for\ntwo out of the nine tests (basics test and tree test) as can be seen here:\n(link: https://github.com/gitgitgadget/git/pull/1698)\n\n-> Other tests:\n\nAs is already mentioned, the rest of the tests in the test directory use the\ncombination of a helper file (written in C) and a shell script that invokes\nthe said helper file. I will use my work from the patch\n‘tests: Move t0009-prio-queue.sh to the unit testing framework’\n(link: https://public-inbox.org/git/pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com/)\nto explain the steps involved in the porting of such tests:\n\n• Search for a suitable test to port: As Christian Couder mentioned in this \nmail (link: https://public-inbox.org/git/CAP8UFD22EpdBU8HJqFM+=75EBABOTf5a0q+KsbzLK+XTEGSkPw@mail.gmail.com/),\nthere exists a subset of t/helper worth porting and we need some sort of\nclassification to discern these.\n\nAll helper files contain a cmd__foo() function which acts as the entry \npoint for that helper tool. For example, the helper/test-prio-queue.c\nfile contained cmd__prio_queue() which served as the entry point for the\nprio-queue tool. This function is then mapped to a different name by \nhelper/test-tool.c which is used by the ‘*.sh’ files to perform tests.\nContinuing the prior example, cmd__prio_queue() had been mapped to\n“prio-queue” by test-tool.c and t0009-prio-queue.sh invoked it like \n“prio-queue 1 2 get 3 dump”.\n\nTo classify what among t/helper should be targeted first in this project\nwe can use something like ‘git grep foo’ (where foo is the alias for cmd__foo())\nto look at the instances where the helper tool is invoked. The ones appearing \nlesser in different test scripts are the ones most likely to be used solely\nfor unit testing and should probably be targeted first.\nUtilising this strategy, I discovered that the ‘prio-queue’ tool was only\nused in t0009-prio-queue.sh and hence, was a good candidate for the unit \ntesting framework.\n\nNote that this strategy is not full-proof and further investigation is\nabsolutely required on a per-test basis, it is only meant to give an\ninitial idea of what’s worth investigating. \n\n•  Create a new C test file in t/unit-tests: After finding a test appropriate\nfor the migration efforts, we create a new ‘*.c’ file in t/unit-tests. \nThe test file must be named appropriately to reflect the nature of the \ntests it is supposed to perform. Most of the times, replacing ‘tXXXX’ \nwith ‘t-‘ and ‘*.sh’ with ‘.c’ in the name of the test script suffices.\nE.g. t/t0009-prio-queue.sh turns to t/unit-tests/t-prio-queue.c. The\nnew C file must #include “test-lib.h” (to be able to use the unit\ntesting framework) and other necessary headers files.\n\n•  Move the code from the helper file: Since the helper files are written\nin C, this step is mostly a ‘copy-paste then rename’ job. Changes similar\nto the following also need to be made in the Makefile:\n-    TEST_BUILTINS_OBJS += test-prio-queue.o\n+    UNIT_TEST_PROGRAMS += t-prio-queue\n\n•  Translate the shell script: The trickiest part of the plan, since \ndifferent test scripts perform various functions, and a direct translation\nof the scripts to C is not always optimal. Continuing the prior example,\nt0009-prio-queue.sh used a single pattern for testing, write expected\noutput to a temporary file (named ‘expect’) -> feed input to the ‘prio-queue’\nhelper tool -> dump its output to another temporary file (named ‘actual’)\n-> compare the two files (‘actual’ vs ‘expect’).\n\nIn the first iteration of my prio-queue patch, I worked out a\nstraightforward translation of this pattern in C. I stored the input in\na string buffer, passed that buffer to the test function and stored its\noutput in another buffer, and then called memcmp() on these two buffers.\nWhile this did prove to be a working copy, this work was found to be inadequate\non the mailing list. Through the next several iterations, I reworked the\nlogic several times, like comparing the input and output on-the-go rather\nthan using buffers and replacing strings with macro definitions.\n\nThe test scripts similarly perform other functions like checking for\nprerequisites, creating commits, initializing repositories, changing or \ncreating directories and so forth, and custom logic is required in most\nof the cases of translating these, as seen above.\n\n•  Run the resulting test, correct any errors: It is rare for any migrated\ntest to work correctly on the first run. This step involves resolving any\ncompile/runtime errors arising from the test and making sure that at the\nvery minimum, all the test-cases of the original test are replicated in the\nnew work. Improvements upon the original can also be made, for example, the\noriginal t0009-prio-queue.sh did not exercise the reverse stack\nfunctionality of prio-queue, which I improved upon in unit-tests/t-prio-queue.\n\n•  Send the resulting patch to the mailing list, respond to the feedback:\nThis step involves writing a meaningful commit message explaining each patch\nin the series. From my experience contributing to the Git project, I find it\nto be rare for any patch series to be accepted in the very first iteration.\nFeedback from the community is vital for the refinement of any patch and\nmust be addressed by performing the suggested changes and sending the work\nback to the mailing list. This must be repeated until the work is merged\ninto ‘seen’, ‘next’ and further down, ‘master’.\n\n\nTimeline\n--------\n\nI’m confident that I can start the project as early as the start of the\nCommunity Bonding Period (May 1 - 26). This is because I have read\nthe related documentation and already have some experience with the idea.\nI believe I’ll be ready to get up to speed to work on the project by then.\nThe exact time arrangement for each test is variable and hard to determine,\nbut judging from the fact that it took me 3-4 days to complete the first\niteration of the t-prio-queue work, here is a proposed migration schedule:\n\n• Reftable tests:\n\nIf my current work from 'tests: Move t0032-reftable-unittest.sh to the unit\ntesting framework' is found satisfactory, there are 7 tests left that need\nto be ported to the unit testing framework. Assuming it takes 2-3 days to\nport one test, I should be done with the first patch series for the reftable\ntests in about 2-3 weeks. From there, it’s a matter of responding to the\nfeedback from the mailing list, which can deceptively take longer than expected.\nFor instance, I had to continue polishing my t-prio-queue patch for about\ntwo weeks after the feedback on the first iteration.\n\n• Other tests:\n\nThe time required to port these tests is highly variable and depends mostly\nupon the work required in translating the shell script. As mentioned\npreviously, it took me 3-4 days to complete the first iteration of the\ntest-prio-queue migration patch, and that was a short test with only about\n50 or so lines of shell scripting and all the test cases following a single\npattern. Considering all this, I believe it should be possible, on average,\nto migrate a new test in 5-8 days.\n\nHence, it should be possible for me to migrate >=7 tests along with the\nreftable tests throughout the duration of this project.\n\nAvailability\n------------\n\nMy summertime is reserved for GSoC, so I expect that I can work on a new \ntest 5 days per week, 6-8 hours per day, that is 30-40 hours a week.\nOn the weekends, I would like to solely work on the feedback from\nthe mailing list and advance my [WIP] patches. Certainly, something\nunexpected may arise, but I will try my best to adhere to this time\ncommitment and be always available through the community’s mailing list.\n\nPost GSoC & Closing Remarks\n---------------------------\n\nWhen I first started contributing to the Git project in October of 2023,\nI had no idea about programmes like GSoC. I was advised by a senior of\nmine to contribute to open-source projects and hence, my aim of contribution\nwas to apply what I had learnt in college to solve real-world problems\nand learn from more experienced peers. However, most of what I have\ncontributed to Git has been trivial owing to my lack of skills and\ninexperience with the project.\n\nSeeing how I need to do an internship in summer, with GSoC, I hope to be\nable to dedicate this internship time and effort to a cool project like\nGit while simultaneously learning skills to be able to make more useful\ncontributions in the future. It’s two birds with one stone. I would also\nlike to keep working on this project to see it to completion post-GSoC\nand help mentor other newcomers get started with the Git project.\n\nThanks & Regards,\nChandra\n"},{"id":"491010","messageId":"CAP8UFD3Rb0kN=g=EPcSqYBb3U=OgW_06jqwafF=vRYhetZ9hdQ@mail.gmail.com","threadId":"61153","inReplyTo":"6f0a3c13-c8d9-4f89-8c62-9c031f0a064e@gmail.com","subject":"Re: [RFC][GSoC] Proposal: Move reftable and other tests to the unit testing framework","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2024-03-20T13:17:23Z","receivedAt":"2024-03-20T13:17:37Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Tue, Mar 19, 2024 at 6:11 PM Chandra Pratap\n<chandrapratap3519@gmail.com> wrote:\n>\n> This is my project proposal for the Google Summer of Code 2024 program.\n> The document version of this proposal can be accessed through this link:\n> https://shorturl.at/ijrTU\n>\n> ---------<8----------<8----------<8----------<8----------<8----------<8\n>\n> Personal Info\n> -------------\n>\n> Full name: Chandra Pratap\n> Preferred name: Chand\n>\n> E-mail: chandrapratap3519@gmail.com\n> Phone: (+91)77618-24030\n>\n> Education: SV National Institute of Technology, Surat, India\n> Year: Sophomore (2nd)\n> Major: Mathematics\n>\n> GitHub: https://github.com/Chand-ra\n>\n> Before GSoC\n> -----------\n>\n> -----Synopsis-----\n>\n> A new unit testing framework was introduced to the Git mailing list last\n> year with the aim of simplifying testing and improving maintainability.\n> The idea was accepted and merged into master on 09/11/2023. This project\n> aims to extend that work by moving more tests from the current setup to\n> the new unit testing framework.\n>\n> The SoC 2024 Ideas page (link: https://git.github.io/SoC-2019-Ideas/)\n> mentions reftable unit tests migration as a separate project from the\n> general unit test migration project, however, I propose migrating other\n> tests alongside the reftable unit tests as a part of this proposal.\n\nIt means that if we select your proposal, we cannot select someone\nelse to work on either the \"Move existing tests to a unit testing\nframework\" project or the \"Convert reftable unit tests to use the unit\ntesting framework\" project.\n\nI am not sure but I think that, after migrating all the reftable unit\ntests, I would prefer you working on other reftable related tasks\nrather than on more unit test migrations.\n\n> The reasoning behind this is explained further down.\n> The difficulty for the project should be medium and it should take\n> somewhat between 175 to 350 hours.\n>\n> -----Contributions-----\n>\n> • apply.c: make git apply respect core.fileMode settings\n> -> Status: merged into master\n> -> link: https://public-inbox.org/git/20231226233218.472054-1-gitster@pobox.com/\n\nA link to (or the hash of) the commit that merged your patch into the\nmaster branch would be nice.\n\n> -> Description: When applying a patch that adds an executable file, git\n> apply ignores the core.fileMode setting (core.fileMode specifies whether\n> the executable bit on files in the working tree are to be honored or not)\n> resulting in false warnings. Fix this by inferring the correct file mode\n> from the existing index entry when core.filemode is set to false. Add a\n> test case that verifies the change and prevents future regression.\n>\n> -> Remarks: This was the first patch I worked on as a contributor to Git.\n> Served me as an essential intro lesson to the community’s working flow and\n> general practices.\n>\n> • tests: Move t0009-prio-queue.sh to the unit testing framework\n> -> Status: merged into master\n> -> link: https://public-inbox.org/git/pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com/\n\nI don't think it's a good idea to submit patches related to a project\nwe propose before the GSoC actually starts. We might have been able to\ndetect this earlier if you added [GSoC] to the patch titles.\n\nAlso a link to (or the hash of) the commit that merged your patch into\nthe master branch would be nice.\n\n> -> Description: t/t0009-prio-queue.sh along with t/helper/test-prio-queue.c\n> unit test Git's implementation of a priority queue. Migrate the test\n> over to the new unit testing framework to simplify debugging and reduce\n> test run-time.\n>\n> -> Remarks: Perhaps the most relevant patch of all the ones mentioned\n> here, this patch helped me understand the expectations and workflow for\n> the work to be performed in this project.\n>\n> • write-or-die: make GIT_FLUSH a Boolean environment variable\n> -> Status: merged into master\n> -> link: https://public-inbox.org/git/pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com/\n\nAlso a link to (or the hash of) the commit that merged your patch into\nthe master branch would be nice.\n\n> -> Description: Among Git's environment variable, the ones marked as\n\ns/variable/variables/\n\n> \"Boolean\" accept values like Boolean configuration variables, i.e.\n> values like 'yes', 'on', 'true' and positive numbers are taken as \"on\"\n> and  values like 'no', 'off','false' are taken as \"off\". Make GIT_FLUSH\n> accept more values besides '0' and '1' by turning it into a Boolean\n> environment variable & update the related documentation.\n>\n> • sideband.c: remoye redundant NEEDSWORK tag\n\ns/remoye/remove/\n\n> -> Status: merged into master\n> -> link: https://public-inbox.org/git/pull.1625.v4.git.1703750460527.gitgitgadget@gmail.com/\n\nAlso a link to (or the hash of) the commit that merged your patch into\nthe master branch would be nice.\n\n> -> Description: Replace a misleading NEEDSWORK tag in sideband.c that\n> reads ‘replace int with size_t’ with another comment explaining why it\n> is fine to use ‘int’ and the replacement isn’t necessary.\n>\n> • make tig callable from PowerShell/Command Prompt\n> -> Status: merged into main\n> -> link: https://github.com/git-for-windows/MINGW-packages/pull/104\n>\n> -> Description: Tig is a text mode interface for Git that ships with the\n> standard Git for Windows package but isn’t callable from PowerShell/\n> Command Prompt by default. Fix this by updating the relevant Makefiles\n> and resource scripts.\n>\n> • fix broken link on Git for Windows’ GitHub wiki\n> -> Status: merged\n> -> link: https://github.com/git-for-windows/git/wiki/Home/_history\n>\n> -> Remarks: A simple fix for a broken link that I stumbled upon while\n> browsing Git for Windows’ wiki looking for some help with the patch\n> mentioned just before this one.\n>\n> • t4129: prevent loss of exit codes due to the use of pipes\n> -> Status: merged into master\n> -> link: https://lore.kernel.org/git/20220311132141.1817-1-shaoxuan.yuan02@gmail.com/\n\nThe link shows a patch from 2022 by Shaoxuan Yuan. Are you sure this\nis the right link?\n\n> -> Description: Piping the output of git commands like git-ls-files to\n> another command (grep in this case) in t4129 hides the exit code returned\n> by these commands. Prevent this by storing the output of git-ls-files to\n> a temporary file and then \"grep-ping\" from that file. Replace grep with\n> test_grep as the latter is more verbose when it fails.\n>\n> • t9146: replace test -d/-f with appropriate test_path_is_* function\n> -> Status: merged into master\n> -> link: https://public-inbox.org/git/pull.1661.v3.git.1707933048210.gitgitgadget@gmail.com/\n\nAlso a link to (or the hash of) the commit that merged your patch into\nthe master branch would be nice.\n\n> -> Description: The helper functions test_path_is_* provide better debugging\n> information than test -d/-e/-f.\n> Replace tests -d/-e/-f with their respective ‘test_path_is_foo’ calls.\n>\n> • regex: update relevant files in compat/regex\n> -> Status: WIP\n> -> link: https://github.com/gitgitgadget/git/pull/1641\n\nThis is a GitGitGadget patch. Please mention that. Same thing for some\nother contributions above that are not part of Git.\n\n> -> Description: The RegEx code in compat/regex has been vendored from\n> gawk and was last updated in 2010. This may lead to performance issues\n> like high CPU usage. Fix this by synchronizing the relevant files in\n> compat/regex with the latest version from GNULib and then replaying any\n> changes we made to gawk’s version on top of the new files.\n>\n> -> Remarks: When I started working on this patch, I thought it was an\n> easy fix but the work turned out to be more involved than I anticipated.\n> I had to seek help from the other community members, and we have made\n> some good progress, but there is still a lot of cleaning and changes that\n> need to be done. I haven’t found enough time to commit to this again,\n> but it’s surely something that I want to get done soon.\n>\n> • tests: Move t0032-reftable-unittest.sh to the unit testing framework\n> -> Status: WIP\n> -> link: https://github.com/gitgitgadget/git/pull/1698\n>\n> -> Description: t/t0032-reftable-unittest.sh along with t/helper/test-reftable.c\n> unit test Git’s reftable framework. Migrate the test over to the new\n> unit testing framework to simplify debugging and reduce test run-time.\n>\n> -> Remarks: An infant version as of now, I tinkered with this after\n> seeing the project list on 'Git SoC 2024 Ideas' page to get an idea of\n> the kind of work that will be involved in this project.\n\nIt's Ok to tinker and to mention this. I hope it helped you write this proposal.\n\n> -----Related Work-----\n>\n> Prior works about the idea have been performed by other community members\n> and previous interns which form a good guiding path for my own approach.\n> Some previous example work:\n>\n> i) Port helper/test-ctype.c to unit-tests/t-ctype.c\n> -> link: https://lore.kernel.org/git/20240112102743.1440-1-ach.lumap@gmail.com/\n>\n> ii) Port test-sha256.c and test-sha1.c to unit-tests/t-hash.c\n> -> link: https://lore.kernel.org/git/20240229054004.3807-2-ach.lumap@gmail.com/\n>\n> iii) Port helper/test-date.c to unit-tests/t-date.c\n> -> link: https://lore.kernel.org/git/20240205162506.1835-2-ach.lumap@gmail.com/\n>\n> iv) Port test-strcmp-offset.c to unit-tests/t-strcmp-offset.c\n> -> link: https://lore.kernel.org/git/20240310144819.4379-1-ach.lumap@gmail.com/\n>\n> v) Integrate a simple strbuf unit test with Git's Makefiles\n> -> link: https://lore.kernel.org/git/20230517-unit-tests-v2-v2-4-21b5b60f4b32@google.com/\n>\n> vi) t0080: turn t-basic unit test into a helper\n> -> link: https://lore.kernel.org/git/a9f67ed703c8314f0f0507ffb83b503717b846b3.1705443632.git.steadmon@google.com/\n\nThanks for mentioning these.\n\n> In GSoC\n> -------\n>\n> -----Plan-----\n>\n> -> Reftable tests:\n>\n> The reftable tests are different from other tests in the test directory\n> because they perform unit testing with the help of a custom test framework\n> rather than the usual ‘helper file + shell script’ combination.\n> Reftable tests do have a helper file and a shell script invoking the\n> helper file, but rather than performing the tests, this combination is\n> used to invoke tests defined in the reftable directory.\n>     The reftable directory consists of nine tests:\n>\n> •  basics test\n> •  record test\n> •  block test\n> •  tree test\n> •  pq test\n> •  stack test\n> •  merged test\n> •  refname test\n> •  read-write test\n>\n> Each of these tests is written in C using a custom reftable testing\n> framework defined by reftable/test_framework (also written in C). The\n> framework has four major features utilized in performing the tests:\n>\n> •  EXPECT_ERR(c): A function-like macro that takes as input an integer\n> ‘c’ (generally the return value of a function call), compares it against\n> 0 and spits an error message if equality doesn’t hold. The error message\n> itself contains information about the file where this macro was used,\n> the line in this file where the macro was called and the error code ‘c’\n> causing the error.\n>\n> •  EXPECT_STREQ(a, b): A function-like macro that takes as input two\n> strings ‘a’ and ‘b’, compares them for equality via strcmp() and throws an\n> error if equality doesn’t hold. The error message thrown contains information\n> regarding the file where this macro was invoked, the line in this\n> file where the macro was called and the mismatched strings ‘a’ and ‘b’.\n>\n> •  EXPECT(c): A function-like macro that takes as input an integer ‘c’\n> (generally the result of a Boolean expression like a == b) and throws an\n> error message if c == 0. The error message is similar to EXPECT_ERR(c).\n>\n> •  RUN_TEST(f): A function-like macro that takes as input the name of a\n> function ‘f’ (a test function that exercises a part of reftable’s code),\n> prints to stdout the message ‘running f’ and then calls the function with f().\n>\n> Other than these, the framework consists of two additional functions,\n> set_test_hash() and strbuf_add_void() which are used  exclusively in the\n> stack tests and refname tests respectively.\n>\n> Since the reftable test framework is written in C like the unit testing\n> framework, we can create a direct translation of the features mentioned\n> above using the existing tools in the unit testing framework with the\n> following plan:\n>\n> •  EXPECT_ERR(c): Can be replaced by check(!c) or check_int(c, “==”, 0).\n>\n> •  EXPECT_STREQ(a, b): Can be replaced by check_str(a, b).\n>\n> •  EXPECT(c): Can be replaced by check_int(), similar to EXPECT_ERR.\n>    E.g. expect(a >= b) --> check_int(a, “>=”, b)\n>\n> •  RUN_TEST(f): Can be replaced by TEST(f(), “message explaining the test”).\n>\n> The information contained in the diagnostic messages of these macros is\n> replicated in the unit testing framework by default. Any additional\n> information can be displayed using the test_msg() functionality in the\n> framework. The additional functions set_test_hash() and strbuf_add_void()\n> may be moved to the source files for stack and refname respectively.\n\nYou mean \"reftable/stack.c\" and \"reftable/refname.c\"?\n\n\n> The plan itself is basic and might need some modifications, but using\n> the above plan, I have already created a working albeit primitive copy for\n> two out of the nine tests (basics test and tree test) as can be seen here:\n> (link: https://github.com/gitgitgadget/git/pull/1698)\n>\n> -> Other tests:\n>\n> As is already mentioned, the rest of the tests in the test directory use the\n> combination of a helper file (written in C) and a shell script that invokes\n> the said helper file. I will use my work from the patch\n> ‘tests: Move t0009-prio-queue.sh to the unit testing framework’\n> (link: https://public-inbox.org/git/pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com/)\n> to explain the steps involved in the porting of such tests:\n>\n> • Search for a suitable test to port: As Christian Couder mentioned in this\n> mail (link: https://public-inbox.org/git/CAP8UFD22EpdBU8HJqFM+=75EBABOTf5a0q+KsbzLK+XTEGSkPw@mail.gmail.com/),\n> there exists a subset of t/helper worth porting and we need some sort of\n> classification to discern these.\n>\n> All helper files contain a cmd__foo() function which acts as the entry\n> point for that helper tool. For example, the helper/test-prio-queue.c\n> file contained cmd__prio_queue() which served as the entry point for the\n> prio-queue tool. This function is then mapped to a different name by\n> helper/test-tool.c which is used by the ‘*.sh’ files to perform tests.\n> Continuing the prior example, cmd__prio_queue() had been mapped to\n> “prio-queue” by test-tool.c and t0009-prio-queue.sh invoked it like\n> “prio-queue 1 2 get 3 dump”.\n>\n> To classify what among t/helper should be targeted first in this project\n> we can use something like ‘git grep foo’ (where foo is the alias for cmd__foo())\n> to look at the instances where the helper tool is invoked. The ones appearing\n> lesser in different test scripts are the ones most likely to be used solely\n> for unit testing and should probably be targeted first.\n> Utilising this strategy, I discovered that the ‘prio-queue’ tool was only\n> used in t0009-prio-queue.sh and hence, was a good candidate for the unit\n> testing framework.\n>\n> Note that this strategy is not full-proof and further investigation is\n> absolutely required on a per-test basis, it is only meant to give an\n> initial idea of what’s worth investigating.\n>\n> •  Create a new C test file in t/unit-tests: After finding a test appropriate\n> for the migration efforts, we create a new ‘*.c’ file in t/unit-tests.\n> The test file must be named appropriately to reflect the nature of the\n> tests it is supposed to perform. Most of the times, replacing ‘tXXXX’\n> with ‘t-‘ and ‘*.sh’ with ‘.c’ in the name of the test script suffices.\n> E.g. t/t0009-prio-queue.sh turns to t/unit-tests/t-prio-queue.c. The\n> new C file must #include “test-lib.h” (to be able to use the unit\n> testing framework) and other necessary headers files.\n>\n> •  Move the code from the helper file: Since the helper files are written\n> in C, this step is mostly a ‘copy-paste then rename’ job. Changes similar\n> to the following also need to be made in the Makefile:\n> -    TEST_BUILTINS_OBJS += test-prio-queue.o\n> +    UNIT_TEST_PROGRAMS += t-prio-queue\n>\n> •  Translate the shell script: The trickiest part of the plan, since\n> different test scripts perform various functions, and a direct translation\n> of the scripts to C is not always optimal. Continuing the prior example,\n> t0009-prio-queue.sh used a single pattern for testing, write expected\n> output to a temporary file (named ‘expect’) -> feed input to the ‘prio-queue’\n> helper tool -> dump its output to another temporary file (named ‘actual’)\n> -> compare the two files (‘actual’ vs ‘expect’).\n>\n> In the first iteration of my prio-queue patch, I worked out a\n> straightforward translation of this pattern in C. I stored the input in\n> a string buffer, passed that buffer to the test function and stored its\n> output in another buffer, and then called memcmp() on these two buffers.\n> While this did prove to be a working copy, this work was found to be inadequate\n> on the mailing list. Through the next several iterations, I reworked the\n> logic several times, like comparing the input and output on-the-go rather\n> than using buffers and replacing strings with macro definitions.\n>\n> The test scripts similarly perform other functions like checking for\n> prerequisites, creating commits, initializing repositories, changing or\n> creating directories and so forth, and custom logic is required in most\n> of the cases of translating these, as seen above.\n>\n> •  Run the resulting test, correct any errors: It is rare for any migrated\n> test to work correctly on the first run. This step involves resolving any\n> compile/runtime errors arising from the test and making sure that at the\n> very minimum, all the test-cases of the original test are replicated in the\n> new work. Improvements upon the original can also be made, for example, the\n> original t0009-prio-queue.sh did not exercise the reverse stack\n> functionality of prio-queue, which I improved upon in unit-tests/t-prio-queue.\n>\n> •  Send the resulting patch to the mailing list, respond to the feedback:\n> This step involves writing a meaningful commit message explaining each patch\n> in the series. From my experience contributing to the Git project, I find it\n> to be rare for any patch series to be accepted in the very first iteration.\n> Feedback from the community is vital for the refinement of any patch and\n> must be addressed by performing the suggested changes and sending the work\n> back to the mailing list. This must be repeated until the work is merged\n> into ‘seen’, ‘next’ and further down, ‘master’.\n>\n>\n> Timeline\n> --------\n>\n> I’m confident that I can start the project as early as the start of the\n> Community Bonding Period (May 1 - 26). This is because I have read\n> the related documentation and already have some experience with the idea.\n> I believe I’ll be ready to get up to speed to work on the project by then.\n> The exact time arrangement for each test is variable and hard to determine,\n> but judging from the fact that it took me 3-4 days to complete the first\n> iteration of the t-prio-queue work, here is a proposed migration schedule:\n>\n> • Reftable tests:\n>\n> If my current work from 'tests: Move t0032-reftable-unittest.sh to the unit\n> testing framework' is found satisfactory, there are 7 tests left that need\n> to be ported to the unit testing framework. Assuming it takes 2-3 days to\n> port one test, I should be done with the first patch series for the reftable\n> tests in about 2-3 weeks. From there, it’s a matter of responding to the\n> feedback from the mailing list, which can deceptively take longer than expected.\n> For instance, I had to continue polishing my t-prio-queue patch for about\n> two weeks after the feedback on the first iteration.\n>\n> • Other tests:\n>\n> The time required to port these tests is highly variable and depends mostly\n> upon the work required in translating the shell script. As mentioned\n> previously, it took me 3-4 days to complete the first iteration of the\n> test-prio-queue migration patch, and that was a short test with only about\n> 50 or so lines of shell scripting and all the test cases following a single\n> pattern. Considering all this, I believe it should be possible, on average,\n> to migrate a new test in 5-8 days.\n>\n> Hence, it should be possible for me to migrate >=7 tests along with the\n> reftable tests throughout the duration of this project.\n>\n> Availability\n> ------------\n>\n> My summertime is reserved for GSoC, so I expect that I can work on a new\n> test 5 days per week, 6-8 hours per day, that is 30-40 hours a week.\n> On the weekends, I would like to solely work on the feedback from\n> the mailing list and advance my [WIP] patches. Certainly, something\n> unexpected may arise, but I will try my best to adhere to this time\n> commitment and be always available through the community’s mailing list.\n>\n> Post GSoC & Closing Remarks\n> ---------------------------\n>\n> When I first started contributing to the Git project in October of 2023,\n> I had no idea about programmes like GSoC. I was advised by a senior of\n> mine to contribute to open-source projects and hence, my aim of contribution\n> was to apply what I had learnt in college to solve real-world problems\n> and learn from more experienced peers. However, most of what I have\n> contributed to Git has been trivial owing to my lack of skills and\n> inexperience with the project.\n>\n> Seeing how I need to do an internship in summer, with GSoC, I hope to be\n> able to dedicate this internship time and effort to a cool project like\n> Git while simultaneously learning skills to be able to make more useful\n> contributions in the future. It’s two birds with one stone. I would also\n> like to keep working on this project to see it to completion post-GSoC\n> and help mentor other newcomers get started with the Git project.\n\n\n> -- After GSoC --\n> ---------------------\n>\n> After GSoC I intend to be a part of the community and keep\n> contributing to the git’s codebase. I eagerly want to learn a lot from\n> the git community and enhance my skills. I also see myself making\n> important contributions to git in the future.\n>\n> When I first joined the community 2 months ago, the ancient way of\n> collaborating through a mailing list by sending diff patches was\n> really puzzling (GitHub was the only means that I knew about for\n> open-source collaboration). After sending a patch I got a little\n> comfortable with it and started loving it.\n>\n> -- Closing remarks --\n> ----------------------------\n>\n> I am really enthusiastic to contribute to git. I really want to learn\n> a lot from my mentors and the whole git community. Everyone\n> contributes using git, why not contribute to git :).\n>\n> In the end I really want to thank the whole community, especially\n> Christian and Junio, for their valuable time in reviewing my patches\n> and guiding me with the problems I faced.\n\nThanks for your proposal. Please make sure you submit it soon as a pdf\nfile to the GSoC website. It can then be updated by uploading a new\npdf until the April 2 1800 UTC deadline.\n"},{"id":"491031","messageId":"2a95b2bc-596a-41bf-b53c-ed9fb39f09ca@gmail.com","threadId":"61153","inReplyTo":"CAP8UFD3Rb0kN=g=EPcSqYBb3U=OgW_06jqwafF=vRYhetZ9hdQ@mail.gmail.com","subject":"Re: [RFC][GSoC] Proposal: Move reftable and other tests to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-03-20T19:35:28Z","receivedAt":"2024-03-20T19:35:40Z","isPatch":false,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"On 20/03/24 18:47, Christian Couder wrote:\n> On Tue, Mar 19, 2024 at 6:11 PM Chandra Pratap\n> <chandrapratap3519@gmail.com> wrote:\n>>\n>> This is my project proposal for the Google Summer of Code 2024 program.\n>> The document version of this proposal can be accessed through this link:\n>> https://shorturl.at/ijrTU\n>>\n>> ---------<8----------<8----------<8----------<8----------<8----------<8\n>>\n>> Personal Info\n>> -------------\n>>\n>> Full name: Chandra Pratap\n>> Preferred name: Chand\n>>\n>> E-mail: chandrapratap3519@gmail.com\n>> Phone: (+91)77618-24030\n>>\n>> Education: SV National Institute of Technology, Surat, India\n>> Year: Sophomore (2nd)\n>> Major: Mathematics\n>>\n>> GitHub: https://github.com/Chand-ra\n>>\n>> Before GSoC\n>> -----------\n>>\n>> -----Synopsis-----\n>>\n>> A new unit testing framework was introduced to the Git mailing list last\n>> year with the aim of simplifying testing and improving maintainability.\n>> The idea was accepted and merged into master on 09/11/2023. This project\n>> aims to extend that work by moving more tests from the current setup to\n>> the new unit testing framework.\n>>\n>> The SoC 2024 Ideas page (link: https://git.github.io/SoC-2019-Ideas/)\n>> mentions reftable unit tests migration as a separate project from the\n>> general unit test migration project, however, I propose migrating other\n>> tests alongside the reftable unit tests as a part of this proposal.\n> \n> It means that if we select your proposal, we cannot select someone\n> else to work on either the \"Move existing tests to a unit testing\n> framework\" project or the \"Convert reftable unit tests to use the unit\n> testing framework\" project.\n> \n> I am not sure but I think that, after migrating all the reftable unit\n> tests, I would prefer you working on other reftable related tasks\n> rather than on more unit test migrations.\n>\n\nSo, should I choose one between the reftable tests migration project and\nthe general unit tests migration project?  Say I decide to go with the\nreftable tests migration project, what kind of \"other reftable related\ntasks\" can I expect to work on? Would it be similar to the day-to-day\nreftable tasks perfomed by contributors or one of the reftable projects\nmentioned on 'SoC 2024 Ideas' page? Would I need to explain these tasks\nin my GSoC proposal?\n \n>> The reasoning behind this is explained further down.\n>> The difficulty for the project should be medium and it should take\n>> somewhat between 175 to 350 hours.\n>>\n>> -----Contributions-----\n>>\n>> • apply.c: make git apply respect core.fileMode settings\n>> -> Status: merged into master\n>> -> link: https://public-inbox.org/git/20231226233218.472054-1-gitster@pobox.com/\n> \n> A link to (or the hash of) the commit that merged your patch into the\n> master branch would be nice.\n> \nAny pointers on how I go about finding this commit/hash?\n\n>> -> Description: When applying a patch that adds an executable file, git\n>> apply ignores the core.fileMode setting (core.fileMode specifies whether\n>> the executable bit on files in the working tree are to be honored or not)\n>> resulting in false warnings. Fix this by inferring the correct file mode\n>> from the existing index entry when core.filemode is set to false. Add a\n>> test case that verifies the change and prevents future regression.\n>>\n>> -> Remarks: This was the first patch I worked on as a contributor to Git.\n>> Served me as an essential intro lesson to the community’s working flow and\n>> general practices.\n>>\n>> • tests: Move t0009-prio-queue.sh to the unit testing framework\n>> -> Status: merged into master\n>> -> link: https://public-inbox.org/git/pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com/\n> \n> I don't think it's a good idea to submit patches related to a project\n> we propose before the GSoC actually starts. We might have been able to\n> detect this earlier if you added [GSoC] to the patch titles.\n> \nAs I mentioned in the 'Closing Remarks' section, this work was done well\nbefore I decided to take part in this year's GSoC (or even before this topic\nwas proposed as a project for this year's GSoC), so I don't think I could\nhave made it a [GSoC] patch.\n\n> Also a link to (or the hash of) the commit that merged your patch into\n> the master branch would be nice.\n> \n\nNoted.\n\n>> -> Description: t/t0009-prio-queue.sh along with t/helper/test-prio-queue.c\n>> unit test Git's implementation of a priority queue. Migrate the test\n>> over to the new unit testing framework to simplify debugging and reduce\n>> test run-time.\n>>\n>> -> Remarks: Perhaps the most relevant patch of all the ones mentioned\n>> here, this patch helped me understand the expectations and workflow for\n>> the work to be performed in this project.\n>>\n>> • write-or-die: make GIT_FLUSH a Boolean environment variable\n>> -> Status: merged into master\n>> -> link: https://public-inbox.org/git/pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com/\n> \n> Also a link to (or the hash of) the commit that merged your patch into\n> the master branch would be nice.\n> \n>> -> Description: Among Git's environment variable, the ones marked as\n> \n> s/variable/variables/\n> \n>> \"Boolean\" accept values like Boolean configuration variables, i.e.\n>> values like 'yes', 'on', 'true' and positive numbers are taken as \"on\"\n>> and  values like 'no', 'off','false' are taken as \"off\". Make GIT_FLUSH\n>> accept more values besides '0' and '1' by turning it into a Boolean\n>> environment variable & update the related documentation.\n>>\n>> • sideband.c: remoye redundant NEEDSWORK tag\n> \n> s/remoye/remove/\n> \n>> -> Status: merged into master\n>> -> link: https://public-inbox.org/git/pull.1625.v4.git.1703750460527.gitgitgadget@gmail.com/\n> \n> Also a link to (or the hash of) the commit that merged your patch into\n> the master branch would be nice.\n> \n>> -> Description: Replace a misleading NEEDSWORK tag in sideband.c that\n>> reads ‘replace int with size_t’ with another comment explaining why it\n>> is fine to use ‘int’ and the replacement isn’t necessary.\n>>\n>> • make tig callable from PowerShell/Command Prompt\n>> -> Status: merged into main\n>> -> link: https://github.com/git-for-windows/MINGW-packages/pull/104\n>>\n>> -> Description: Tig is a text mode interface for Git that ships with the\n>> standard Git for Windows package but isn’t callable from PowerShell/\n>> Command Prompt by default. Fix this by updating the relevant Makefiles\n>> and resource scripts.\n>>\n>> • fix broken link on Git for Windows’ GitHub wiki\n>> -> Status: merged\n>> -> link: https://github.com/git-for-windows/git/wiki/Home/_history\n>>\n>> -> Remarks: A simple fix for a broken link that I stumbled upon while\n>> browsing Git for Windows’ wiki looking for some help with the patch\n>> mentioned just before this one.\n>>\n>> • t4129: prevent loss of exit codes due to the use of pipes\n>> -> Status: merged into master\n>> -> link: https://lore.kernel.org/git/20220311132141.1817-1-shaoxuan.yuan02@gmail.com/\n> \n> The link shows a patch from 2022 by Shaoxuan Yuan. Are you sure this\n> is the right link?\n\nThat seems to be an error. I will make sure to correct it in the next draft.\n\n> \n>> -> Description: Piping the output of git commands like git-ls-files to\n>> another command (grep in this case) in t4129 hides the exit code returned\n>> by these commands. Prevent this by storing the output of git-ls-files to\n>> a temporary file and then \"grep-ping\" from that file. Replace grep with\n>> test_grep as the latter is more verbose when it fails.\n>>\n>> • t9146: replace test -d/-f with appropriate test_path_is_* function\n>> -> Status: merged into master\n>> -> link: https://public-inbox.org/git/pull.1661.v3.git.1707933048210.gitgitgadget@gmail.com/\n> \n> Also a link to (or the hash of) the commit that merged your patch into\n> the master branch would be nice.\n> \n>> -> Description: The helper functions test_path_is_* provide better debugging\n>> information than test -d/-e/-f.\n>> Replace tests -d/-e/-f with their respective ‘test_path_is_foo’ calls.\n>>\n>> • regex: update relevant files in compat/regex\n>> -> Status: WIP\n>> -> link: https://github.com/gitgitgadget/git/pull/1641\n> \n> This is a GitGitGadget patch. Please mention that. Same thing for some\n> other contributions above that are not part of Git.\n> \n\nSure thing.\n\n>> -> Description: The RegEx code in compat/regex has been vendored from\n>> gawk and was last updated in 2010. This may lead to performance issues\n>> like high CPU usage. Fix this by synchronizing the relevant files in\n>> compat/regex with the latest version from GNULib and then replaying any\n>> changes we made to gawk’s version on top of the new files.\n>>\n>> -> Remarks: When I started working on this patch, I thought it was an\n>> easy fix but the work turned out to be more involved than I anticipated.\n>> I had to seek help from the other community members, and we have made\n>> some good progress, but there is still a lot of cleaning and changes that\n>> need to be done. I haven’t found enough time to commit to this again,\n>> but it’s surely something that I want to get done soon.\n>>\n>> • tests: Move t0032-reftable-unittest.sh to the unit testing framework\n>> -> Status: WIP\n>> -> link: https://github.com/gitgitgadget/git/pull/1698\n>>\n>> -> Description: t/t0032-reftable-unittest.sh along with t/helper/test-reftable.c\n>> unit test Git’s reftable framework. Migrate the test over to the new\n>> unit testing framework to simplify debugging and reduce test run-time.\n>>\n>> -> Remarks: An infant version as of now, I tinkered with this after\n>> seeing the project list on 'Git SoC 2024 Ideas' page to get an idea of\n>> the kind of work that will be involved in this project.\n> \n> It's Ok to tinker and to mention this. I hope it helped you write this proposal.\n> \n>> -----Related Work-----\n>>\n>> Prior works about the idea have been performed by other community members\n>> and previous interns which form a good guiding path for my own approach.\n>> Some previous example work:\n>>\n>> i) Port helper/test-ctype.c to unit-tests/t-ctype.c\n>> -> link: https://lore.kernel.org/git/20240112102743.1440-1-ach.lumap@gmail.com/\n>>\n>> ii) Port test-sha256.c and test-sha1.c to unit-tests/t-hash.c\n>> -> link: https://lore.kernel.org/git/20240229054004.3807-2-ach.lumap@gmail.com/\n>>\n>> iii) Port helper/test-date.c to unit-tests/t-date.c\n>> -> link: https://lore.kernel.org/git/20240205162506.1835-2-ach.lumap@gmail.com/\n>>\n>> iv) Port test-strcmp-offset.c to unit-tests/t-strcmp-offset.c\n>> -> link: https://lore.kernel.org/git/20240310144819.4379-1-ach.lumap@gmail.com/\n>>\n>> v) Integrate a simple strbuf unit test with Git's Makefiles\n>> -> link: https://lore.kernel.org/git/20230517-unit-tests-v2-v2-4-21b5b60f4b32@google.com/\n>>\n>> vi) t0080: turn t-basic unit test into a helper\n>> -> link: https://lore.kernel.org/git/a9f67ed703c8314f0f0507ffb83b503717b846b3.1705443632.git.steadmon@google.com/\n> \n> Thanks for mentioning these.\n> \n\n:)\n\n>> In GSoC\n>> -------\n>>\n>> -----Plan-----\n>>\n>> -> Reftable tests:\n>>\n>> The reftable tests are different from other tests in the test directory\n>> because they perform unit testing with the help of a custom test framework\n>> rather than the usual ‘helper file + shell script’ combination.\n>> Reftable tests do have a helper file and a shell script invoking the\n>> helper file, but rather than performing the tests, this combination is\n>> used to invoke tests defined in the reftable directory.\n>>     The reftable directory consists of nine tests:\n>>\n>> •  basics test\n>> •  record test\n>> •  block test\n>> •  tree test\n>> •  pq test\n>> •  stack test\n>> •  merged test\n>> •  refname test\n>> •  read-write test\n>>\n>> Each of these tests is written in C using a custom reftable testing\n>> framework defined by reftable/test_framework (also written in C). The\n>> framework has four major features utilized in performing the tests:\n>>\n>> •  EXPECT_ERR(c): A function-like macro that takes as input an integer\n>> ‘c’ (generally the return value of a function call), compares it against\n>> 0 and spits an error message if equality doesn’t hold. The error message\n>> itself contains information about the file where this macro was used,\n>> the line in this file where the macro was called and the error code ‘c’\n>> causing the error.\n>>\n>> •  EXPECT_STREQ(a, b): A function-like macro that takes as input two\n>> strings ‘a’ and ‘b’, compares them for equality via strcmp() and throws an\n>> error if equality doesn’t hold. The error message thrown contains information\n>> regarding the file where this macro was invoked, the line in this\n>> file where the macro was called and the mismatched strings ‘a’ and ‘b’.\n>>\n>> •  EXPECT(c): A function-like macro that takes as input an integer ‘c’\n>> (generally the result of a Boolean expression like a == b) and throws an\n>> error message if c == 0. The error message is similar to EXPECT_ERR(c).\n>>\n>> •  RUN_TEST(f): A function-like macro that takes as input the name of a\n>> function ‘f’ (a test function that exercises a part of reftable’s code),\n>> prints to stdout the message ‘running f’ and then calls the function with f().\n>>\n>> Other than these, the framework consists of two additional functions,\n>> set_test_hash() and strbuf_add_void() which are used  exclusively in the\n>> stack tests and refname tests respectively.\n>>\n>> Since the reftable test framework is written in C like the unit testing\n>> framework, we can create a direct translation of the features mentioned\n>> above using the existing tools in the unit testing framework with the\n>> following plan:\n>>\n>> •  EXPECT_ERR(c): Can be replaced by check(!c) or check_int(c, “==”, 0).\n>>\n>> •  EXPECT_STREQ(a, b): Can be replaced by check_str(a, b).\n>>\n>> •  EXPECT(c): Can be replaced by check_int(), similar to EXPECT_ERR.\n>>    E.g. expect(a >= b) --> check_int(a, “>=”, b)\n>>\n>> •  RUN_TEST(f): Can be replaced by TEST(f(), “message explaining the test”).\n>>\n>> The information contained in the diagnostic messages of these macros is\n>> replicated in the unit testing framework by default. Any additional\n>> information can be displayed using the test_msg() functionality in the\n>> framework. The additional functions set_test_hash() and strbuf_add_void()\n>> may be moved to the source files for stack and refname respectively.\n> \n> You mean \"reftable/stack.c\" and \"reftable/refname.c\"?\n> \n\nYes, I will re-write this to make it clearer.\n\n> \n>> The plan itself is basic and might need some modifications, but using\n>> the above plan, I have already created a working albeit primitive copy for\n>> two out of the nine tests (basics test and tree test) as can be seen here:\n>> (link: https://github.com/gitgitgadget/git/pull/1698)\n>>\n>> -> Other tests:\n>>\n>> As is already mentioned, the rest of the tests in the test directory use the\n>> combination of a helper file (written in C) and a shell script that invokes\n>> the said helper file. I will use my work from the patch\n>> ‘tests: Move t0009-prio-queue.sh to the unit testing framework’\n>> (link: https://public-inbox.org/git/pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com/)\n>> to explain the steps involved in the porting of such tests:\n>>\n>> • Search for a suitable test to port: As Christian Couder mentioned in this\n>> mail (link: https://public-inbox.org/git/CAP8UFD22EpdBU8HJqFM+=75EBABOTf5a0q+KsbzLK+XTEGSkPw@mail.gmail.com/),\n>> there exists a subset of t/helper worth porting and we need some sort of\n>> classification to discern these.\n>>\n>> All helper files contain a cmd__foo() function which acts as the entry\n>> point for that helper tool. For example, the helper/test-prio-queue.c\n>> file contained cmd__prio_queue() which served as the entry point for the\n>> prio-queue tool. This function is then mapped to a different name by\n>> helper/test-tool.c which is used by the ‘*.sh’ files to perform tests.\n>> Continuing the prior example, cmd__prio_queue() had been mapped to\n>> “prio-queue” by test-tool.c and t0009-prio-queue.sh invoked it like\n>> “prio-queue 1 2 get 3 dump”.\n>>\n>> To classify what among t/helper should be targeted first in this project\n>> we can use something like ‘git grep foo’ (where foo is the alias for cmd__foo())\n>> to look at the instances where the helper tool is invoked. The ones appearing\n>> lesser in different test scripts are the ones most likely to be used solely\n>> for unit testing and should probably be targeted first.\n>> Utilising this strategy, I discovered that the ‘prio-queue’ tool was only\n>> used in t0009-prio-queue.sh and hence, was a good candidate for the unit\n>> testing framework.\n>>\n>> Note that this strategy is not full-proof and further investigation is\n>> absolutely required on a per-test basis, it is only meant to give an\n>> initial idea of what’s worth investigating.\n>>\n>> •  Create a new C test file in t/unit-tests: After finding a test appropriate\n>> for the migration efforts, we create a new ‘*.c’ file in t/unit-tests.\n>> The test file must be named appropriately to reflect the nature of the\n>> tests it is supposed to perform. Most of the times, replacing ‘tXXXX’\n>> with ‘t-‘ and ‘*.sh’ with ‘.c’ in the name of the test script suffices.\n>> E.g. t/t0009-prio-queue.sh turns to t/unit-tests/t-prio-queue.c. The\n>> new C file must #include “test-lib.h” (to be able to use the unit\n>> testing framework) and other necessary headers files.\n>>\n>> •  Move the code from the helper file: Since the helper files are written\n>> in C, this step is mostly a ‘copy-paste then rename’ job. Changes similar\n>> to the following also need to be made in the Makefile:\n>> -    TEST_BUILTINS_OBJS += test-prio-queue.o\n>> +    UNIT_TEST_PROGRAMS += t-prio-queue\n>>\n>> •  Translate the shell script: The trickiest part of the plan, since\n>> different test scripts perform various functions, and a direct translation\n>> of the scripts to C is not always optimal. Continuing the prior example,\n>> t0009-prio-queue.sh used a single pattern for testing, write expected\n>> output to a temporary file (named ‘expect’) -> feed input to the ‘prio-queue’\n>> helper tool -> dump its output to another temporary file (named ‘actual’)\n>> -> compare the two files (‘actual’ vs ‘expect’).\n>>\n>> In the first iteration of my prio-queue patch, I worked out a\n>> straightforward translation of this pattern in C. I stored the input in\n>> a string buffer, passed that buffer to the test function and stored its\n>> output in another buffer, and then called memcmp() on these two buffers.\n>> While this did prove to be a working copy, this work was found to be inadequate\n>> on the mailing list. Through the next several iterations, I reworked the\n>> logic several times, like comparing the input and output on-the-go rather\n>> than using buffers and replacing strings with macro definitions.\n>>\n>> The test scripts similarly perform other functions like checking for\n>> prerequisites, creating commits, initializing repositories, changing or\n>> creating directories and so forth, and custom logic is required in most\n>> of the cases of translating these, as seen above.\n>>\n>> •  Run the resulting test, correct any errors: It is rare for any migrated\n>> test to work correctly on the first run. This step involves resolving any\n>> compile/runtime errors arising from the test and making sure that at the\n>> very minimum, all the test-cases of the original test are replicated in the\n>> new work. Improvements upon the original can also be made, for example, the\n>> original t0009-prio-queue.sh did not exercise the reverse stack\n>> functionality of prio-queue, which I improved upon in unit-tests/t-prio-queue.\n>>\n>> •  Send the resulting patch to the mailing list, respond to the feedback:\n>> This step involves writing a meaningful commit message explaining each patch\n>> in the series. From my experience contributing to the Git project, I find it\n>> to be rare for any patch series to be accepted in the very first iteration.\n>> Feedback from the community is vital for the refinement of any patch and\n>> must be addressed by performing the suggested changes and sending the work\n>> back to the mailing list. This must be repeated until the work is merged\n>> into ‘seen’, ‘next’ and further down, ‘master’.\n>>\n>>\n>> Timeline\n>> --------\n>>\n>> I’m confident that I can start the project as early as the start of the\n>> Community Bonding Period (May 1 - 26). This is because I have read\n>> the related documentation and already have some experience with the idea.\n>> I believe I’ll be ready to get up to speed to work on the project by then.\n>> The exact time arrangement for each test is variable and hard to determine,\n>> but judging from the fact that it took me 3-4 days to complete the first\n>> iteration of the t-prio-queue work, here is a proposed migration schedule:\n>>\n>> • Reftable tests:\n>>\n>> If my current work from 'tests: Move t0032-reftable-unittest.sh to the unit\n>> testing framework' is found satisfactory, there are 7 tests left that need\n>> to be ported to the unit testing framework. Assuming it takes 2-3 days to\n>> port one test, I should be done with the first patch series for the reftable\n>> tests in about 2-3 weeks. From there, it’s a matter of responding to the\n>> feedback from the mailing list, which can deceptively take longer than expected.\n>> For instance, I had to continue polishing my t-prio-queue patch for about\n>> two weeks after the feedback on the first iteration.\n>>\n>> • Other tests:\n>>\n>> The time required to port these tests is highly variable and depends mostly\n>> upon the work required in translating the shell script. As mentioned\n>> previously, it took me 3-4 days to complete the first iteration of the\n>> test-prio-queue migration patch, and that was a short test with only about\n>> 50 or so lines of shell scripting and all the test cases following a single\n>> pattern. Considering all this, I believe it should be possible, on average,\n>> to migrate a new test in 5-8 days.\n>>\n>> Hence, it should be possible for me to migrate >=7 tests along with the\n>> reftable tests throughout the duration of this project.\n>>\n>> Availability\n>> ------------\n>>\n>> My summertime is reserved for GSoC, so I expect that I can work on a new\n>> test 5 days per week, 6-8 hours per day, that is 30-40 hours a week.\n>> On the weekends, I would like to solely work on the feedback from\n>> the mailing list and advance my [WIP] patches. Certainly, something\n>> unexpected may arise, but I will try my best to adhere to this time\n>> commitment and be always available through the community’s mailing list.\n>>\n>> Post GSoC & Closing Remarks\n>> ---------------------------\n>>\n>> When I first started contributing to the Git project in October of 2023,\n>> I had no idea about programmes like GSoC. I was advised by a senior of\n>> mine to contribute to open-source projects and hence, my aim of contribution\n>> was to apply what I had learnt in college to solve real-world problems\n>> and learn from more experienced peers. However, most of what I have\n>> contributed to Git has been trivial owing to my lack of skills and\n>> inexperience with the project.\n>>\n>> Seeing how I need to do an internship in summer, with GSoC, I hope to be\n>> able to dedicate this internship time and effort to a cool project like\n>> Git while simultaneously learning skills to be able to make more useful\n>> contributions in the future. It’s two birds with one stone. I would also\n>> like to keep working on this project to see it to completion post-GSoC\n>> and help mentor other newcomers get started with the Git project.\n> \n> \n>> -- After GSoC --\n>> ---------------------\n>>\n>> After GSoC I intend to be a part of the community and keep\n>> contributing to the git’s codebase. I eagerly want to learn a lot from\n>> the git community and enhance my skills. I also see myself making\n>> important contributions to git in the future.\n>>\n>> When I first joined the community 2 months ago, the ancient way of\n>> collaborating through a mailing list by sending diff patches was\n>> really puzzling (GitHub was the only means that I knew about for\n>> open-source collaboration). After sending a patch I got a little\n>> comfortable with it and started loving it.\n>>\n>> -- Closing remarks --\n>> ----------------------------\n>>\n>> I am really enthusiastic to contribute to git. I really want to learn\n>> a lot from my mentors and the whole git community. Everyone\n>> contributes using git, why not contribute to git :).\n>>\n>> In the end I really want to thank the whole community, especially\n>> Christian and Junio, for their valuable time in reviewing my patches\n>> and guiding me with the problems I faced.\n> \n> Thanks for your proposal. Please make sure you submit it soon as a pdf\n> file to the GSoC website. It can then be updated by uploading a new\n> pdf until the April 2 1800 UTC deadline.\n\nThanks for the feedback, I will.\n"},{"id":"491122","messageId":"ZfwnrL6Zl_lcV09y@tanuki","threadId":"61153","inReplyTo":"CAP8UFD3Rb0kN=g=EPcSqYBb3U=OgW_06jqwafF=vRYhetZ9hdQ@mail.gmail.com","subject":"Re: [RFC][GSoC] Proposal: Move reftable and other tests to the unit testing framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-03-21T12:27:24Z","receivedAt":"2024-03-21T12:27:32Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Mar 20, 2024 at 02:17:23PM +0100, Christian Couder wrote:\n> On Tue, Mar 19, 2024 at 6:11 PM Chandra Pratap\n> <chandrapratap3519@gmail.com> wrote:\n[snip]\n> > A new unit testing framework was introduced to the Git mailing list last\n> > year with the aim of simplifying testing and improving maintainability.\n> > The idea was accepted and merged into master on 09/11/2023. This project\n> > aims to extend that work by moving more tests from the current setup to\n> > the new unit testing framework.\n> >\n> > The SoC 2024 Ideas page (link: https://git.github.io/SoC-2019-Ideas/)\n> > mentions reftable unit tests migration as a separate project from the\n> > general unit test migration project, however, I propose migrating other\n> > tests alongside the reftable unit tests as a part of this proposal.\n> \n> It means that if we select your proposal, we cannot select someone\n> else to work on either the \"Move existing tests to a unit testing\n> framework\" project or the \"Convert reftable unit tests to use the unit\n> testing framework\" project.\n> \n> I am not sure but I think that, after migrating all the reftable unit\n> tests, I would prefer you working on other reftable related tasks\n> rather than on more unit test migrations.\n\nI agree, I'd also like to keep these projects separate from each other.\nAlso, the reftable tests could certainly use some polishing and a lot\nmore documentation than they currently have. Their coding style does not\nreally match the rest of the project, and if we're busy migrating the\ncode then I think we should also take the chance and touch it up.\n\nThat will require the student to understand the reftable code a whole\nlot more deeply than a mere conversion would require though. Thus, I\nwould like to caution any student who wants to pick up this project to\nnot underestimate the effort required for this project. The reftable\ncode itself is certainly non-trivial.\n\nAs Chris mentioned, if this project would be finished early I'm quite\nsure that the efforts would uncover bugs, test gaps or similar things in\nthe vicinity of reftables that the student could certainly continue to\nwork on.\n\nPatrick\n"},{"id":"491816","messageId":"e2d91c9f-9c7a-4127-9bc2-41807765241b@gmail.com","threadId":"61153","inReplyTo":"ZfwnrL6Zl_lcV09y@tanuki","subject":"Re: [RFC][GSoC] Proposal v2: Move more tests to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-03-29T06:54:49Z","receivedAt":"2024-03-29T06:54:55Z","isPatch":false,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Thanks for the feedback, Christian and Patrick! With your advice, I\ndecided to split my original proposal into two to conform to what was\nsuggested by the SoC 2024 Ideas page.\nThis is the proposal for the unit tests migration project.\n\n---------<8----------<8----------<8----------<8----------<8----------<8\n\nPersonal Info\n-------------\n\nFull name: Chandra Pratap\nPreferred name: Chand\n\nE-mail: chandrapratap3519@gmail.com\nPhone: (+91)77618-24030\n\nEducation: SV National Institute of Technology, Surat, India\nYear: Sophomore (2nd)\nMajor: Mathematics\n\nGitHub: https://github.com/Chand-ra\n\nBefore GSoC\n-----------\n\n-----Synopsis-----\n\nA new unit testing framework was introduced to the Git mailing list last\nyear with the aim of simplifying testing and improving maintainability\nby moving the current testing setup from shell scripts and helper files\nto a framework written wholly in C. The idea was accepted and merged\ninto master on 09/11/2023. The choice of testing framework and the\nreasoning behind the choice is described in\nDocumentation/technical/unit-tests.txt.\n\nThis project aims to extend that work by moving more tests from the current\nsetup to the new unit testing framework. The difficulty for the project\nshould be medium and it should take somewhat between 175 to 350 hours.\n\n-----Contributions-----\n\n• apply.c: make git apply respect core.fileMode settings\n-> Status: merged into master\n-> link: https://public-inbox.org/git/20231226233218.472054-1-gitster@pobox.com/\n-> Merge commit: cf47fb7ec7e183a1a1e521a540862fba3c2a89eb\n\n-> Description: When applying a patch that adds an executable file, git \napply ignores the core.fileMode setting (core.fileMode specifies whether\nthe executable bit on files in the working tree are to be honored or not)\nresulting in false warnings. Fix this by inferring the correct file mode \nfrom the existing index entry when core.filemode is set to false. Add a\ntest case that verifies the change and prevents future regression.\n\n-> Remarks: This was the first patch I worked on as a contributor to Git.\nServed me as an essential intro lesson to the community’s working flow and\ngeneral practices.\n\n• tests: Move t0009-prio-queue.sh to the unit testing framework\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com/\n-> Merge commit: 107023e1c9f981476c505e73eab319db6534a536\n\n-> Description: t/t0009-prio-queue.sh along with t/helper/test-prio-queue.c\nunit test Git's implementation of a priority queue. Migrate the test\nover to the new unit testing framework to simplify debugging and reduce\ntest run-time.\n\n-> Remarks: Probably the most relevant patch of all the ones mentioned\nhere, I decided to work on this patch well before I decided to take part\nin this year’s GSoC. This patch helped me understand the expectations and\nworkflow for the work to be performed in unit tests migrations.\n\n• write-or-die: make GIT_FLUSH a Boolean environment variable\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com/\n-> Merge commit: b3049bbb97c9c0d0292bc9239e976cc661961f39\n\n-> Description: Among Git's environment variables, the ones marked as\n\"Boolean\" accept values like Boolean configuration variables, i.e. \nvalues like 'yes', 'on', 'true' and positive numbers are taken as \"on\"\nand  values like 'no', 'off','false' are taken as \"off\". Make GIT_FLUSH \naccept more values besides '0' and '1' by turning it into a Boolean \nenvironment variable & update the related documentation.\n\n• sideband.c: remoye redundant NEEDSWORK tag\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1625.v4.git.1703750460527.gitgitgadget@gmail.com/\n-> Merge commit: 6484eb9a97fe3cd81a2d5711183b93494e6ddefa\n\n-> Description: Replace a misleading NEEDSWORK tag in sideband.c that\nreads ‘replace int with size_t’ with another comment explaining why it\nis fine to use ‘int’ and the replacement isn’t necessary.\n\n• make tig callable from PowerShell/Command Prompt\n-> Status: merged into main (Git for Windows)\n-> link: https://github.com/git-for-windows/MINGW-packages/pull/104\n-> Merge commit: 8eb71eb31c6a1b5d41b253c1ab66d981cc448cb0\n\n-> Description: Tig is a text mode interface for Git that ships with the\nstandard Git for Windows package but isn’t callable from PowerShell/\nCommand Prompt by default. Fix this by updating the relevant Makefiles\nand resource scripts.\n\n• fix broken link on Git for Windows’ GitHub wiki\n-> Status: merged (Git for Windows)\n-> link: https://github.com/git-for-windows/git/wiki/Home/_history\n-> Merge commit: 0df663304bce986da6571cca48b34508d4823a11\n\n-> Remarks: A simple fix for a broken link that I stumbled upon while\nbrowsing Git for Windows’ wiki looking for some help with the patch \nmentioned just before this one.\n\n• t4129: prevent loss of exit codes due to the use of pipes\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1636.git.1704891257544.gitgitgadget@gmail.com/\n-> Merge commit: 1b095626937a858e3a68e9c7d1de11b71117bb42\n\n-> Description: Piping the output of git commands like git-ls-files to\nanother command (grep in this case) in t4129 hides the exit code returned\nby these commands. Prevent this by storing the output of git-ls-files to\na temporary file and then \"grep-ping\" from that file. Replace grep with\ntest_grep as the latter is more verbose when it fails.\n\n• t9146: replace test -d/-f with appropriate test_path_is_* function\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1661.v3.git.1707933048210.gitgitgadget@gmail.com/\n-> Merge commit: 90c0c15e56fa761ae8c4cf5f5fe09b329c0a5dc5\n\n-> Description: The helper functions test_path_is_* provide better debugging\ninformation than test -d/-e/-f.\nReplace tests -d/-e/-f with their respective ‘test_path_is_foo’ calls.\n\n• regex: update relevant files in compat/regex\n-> Status: WIP (GitGitGadget PR)\n-> link: https://github.com/gitgitgadget/git/pull/1641\n\n-> Description: The RegEx code in compat/regex has been vendored from \ngawk and was last updated in 2010. This may lead to performance issues \nlike high CPU usage. Fix this by synchronizing the relevant files in \ncompat/regex with the latest version from GNULib and then replaying any\nchanges we made to gawk’s version on top of the new files.\n\n-> Remarks: When I started working on this patch, I thought it was an\neasy fix but the work turned out to be more involved than I anticipated.\nI had to seek help from the other community members, and we have made\nsome good progress, but there is still a lot of cleaning and changes that\nneed to be done. I haven’t found enough time to commit to this again,\nbut it’s surely something that I want to get done soon.\n\n• tests: Move t0032-reftable-unittest.sh to the unit testing framework\n-> Status: WIP (GitGitGadget PR)\n-> link: https://github.com/gitgitgadget/git/pull/1698\n\n-> Description: t/t0032-reftable-unittest.sh along with t/helper/test-reftable.c\nunit test Git’s reftable framework. Migrate the test over to the new\nunit testing framework to simplify debugging and reduce test run-time.\n\n-> Remarks: An infant version as of now, I tinkered with this after \nseeing the project list on 'Git SoC 2024 Ideas' page to get an idea of\nthe kind of work that will be involved in the ‘Move reftable tests to\nthe new unit testing framework’ project.\n\n• commit.c: ensure find_header_mem() doesn't scan beyond given range\n-> Status: Dropped\n\n-> Remarks: This was a patch addressing a NEEDSWORK comment in commit.c\nwhich was dropped because René Scharfe found out a better way to fix the\nissue at hand than my approach or what the NEEDSWORK comment suggested.\n\n\n-----Related Work-----\n\nPrior works about the idea have been performed by other community members\nand previous interns which form a good guiding path for my own approach.\nSome previous example work:\n\ni) Port helper/test-ctype.c to unit-tests/t-ctype.c\n-> link: https://lore.kernel.org/git/20240112102743.1440-1-ach.lumap@gmail.com/\n\nii) Port test-sha256.c and test-sha1.c to unit-tests/t-hash.c\n-> link: https://lore.kernel.org/git/20240229054004.3807-2-ach.lumap@gmail.com/\n\niii) Port helper/test-date.c to unit-tests/t-date.c\n-> link: https://lore.kernel.org/git/20240205162506.1835-2-ach.lumap@gmail.com/\n\niv) Port test-strcmp-offset.c to unit-tests/t-strcmp-offset.c\n-> link: https://lore.kernel.org/git/20240310144819.4379-1-ach.lumap@gmail.com/\n\nv) Integrate a simple strbuf unit test with Git's Makefiles\n-> link: https://lore.kernel.org/git/20230517-unit-tests-v2-v2-4-21b5b60f4b32@google.com/\n\nvi) t0080: turn t-basic unit test into a helper\n-> link: https://lore.kernel.org/git/a9f67ed703c8314f0f0507ffb83b503717b846b3.1705443632.git.steadmon@google.com/\n\nIn GSoC\n-------\n\n-----Plan-----\n\nTests for Git are defined in the t/ directory and use the combination of\na helper file (written in C) and a shell script that invokes the said\nhelper file. I will use my work from the patch ‘tests: Move\nt0009-prio-queue.sh to the unit testing framework’ to explain the steps\ninvolved in the porting of such tests:\n\n• Search for a suitable test to port: \n\nAs Christian Couder mentioned in this mail (link: https://public-inbox.org/git/CAP8UFD22EpdBU8HJqFM+=75EBABOTf5a0q+KsbzLK+XTEGSkPw@mail.gmail.com/),\nthere exists a subset of t/helper worth porting and we need some sort of\nclassification to discern these.\n\nAll helper files contain a cmd__foo() function which acts as the entry\npoint for that helper tool. For example, the helper/test-prio-queue.c\nfile contained cmd__prio_queue() which served as the entry point for\nthat file. The binary for the helper file is then mapped to a different\nname by helper/test-tool.c which is used by the ‘*.sh’ files to perform\nthe tests. This name can be discovered by searching for the helper\nfile’s entry point in test-tool.c. Continuing the prior example,\n“prio-queue” was the name for the helper/test-prio-queue.c binary and\nt0009-prio-queue.sh invoked it like “prio-queue 1 2 get 3 dump”.\n\nTo classify what among t/helper should be targeted first in this\nproject, we can use something like ‘git grep foo’ (where foo is the name\nfor the helper’s binary) to look at the instances where the helper tool\nis invoked. The ones appearing lesser in different test scripts are the\nones most likely to be used solely for unit testing and should probably\nbe targeted first. Utilising this strategy, I discovered that the\n‘prio-queue’ tool was only used in t0009-prio-queue.sh and hence, was a\ngood candidate for the unit testing framework.\nNote that this strategy is not full-proof and further investigation is\nabsolutely required on a per-test basis, it is only meant to give an\ninitial idea of what’s worth investigating.\n\n•  Create a new C test file in t/unit-tests: \n\nAfter finding a test appropriate for the migration efforts, we create a\nnew ‘*.c’ file in t/unit-tests.  The test file must be named appropriately\nto reflect the nature of the  tests it is supposed to perform.\nMost of the times, replacing ‘tXXXX’  with ‘t-‘ and ‘*.sh’ with ‘.c’ in\nthe name of the test script suffices. E.g. t/t0009-prio-queue.sh turns\nto t/unit-tests/t-prio-queue.c.\nThe new C file must #include “test-lib.h” (to be able to use the unit\ntesting framework) and other necessary headers files.\n\n•  Move the code from the helper file: \n\nSince the helper files are written in C, this step is mostly a\n‘copy-paste then rename’ job. Changes similar to the following also need\nto be made in the Makefile:\n-    TEST_BUILTINS_OBJS += test-prio-queue.o\n+    UNIT_TEST_PROGRAMS += t-prio-queue\n\n•  Translate the shell script: \n\nThe trickiest part of the plan, since  different test scripts perform\nvarious functions and a direct translation of the scripts to C is not\nalways optimal. Continuing the prior example, t0009-prio-queue.sh used a\nsingle pattern for testing, write expected output to a temporary file\n(named ‘expect’) -> feed input to the ‘prio-queue’ helper tool -> dump its\noutput to another temporary file (named ‘actual’) -> compare the two files\n(‘actual’ vs ‘expect’).\n\nIn the first iteration of my prio-queue patch, I worked out a\nstraightforward translation of this pattern in C. I stored the input in\na string buffer, passed that buffer to the test function, stored its\noutput in another buffer and then called memcmp() on these two buffers.\nWhile this did prove to be a working copy, this work was found to be inadequate\non the mailing list. Through the next several iterations, I reworked the\nlogic several times, like comparing the input and output on-the-go rather\nthan using buffers and replacing strings with macro definitions.\n\nThe test scripts similarly perform other functions like checking for\nprerequisites, creating commits, initializing repositories, changing or \ncreating directories and so forth, and custom logic is required in most\nof the cases of translating these, as seen above.\n\n•  Run the resulting test, correct any errors:\n\nIt is rare for any migrated test to work correctly on the first run.\nThis step involves resolving any compile/runtime errors arising from the\ntest and making sure that at the very minimum, all the test-cases of the\noriginal test are replicated in the new work. Improvements upon the original\ncan also be made, for example, the original t0009-prio-queue.sh did not\nexercise the reverse stack functionality of prio-queue, which I improved\nupon in unit-tests/t-prio-queue.\n\n•  Send the resulting patch to the mailing list, respond to the feedback:\n\nThis step involves writing a meaningful commit message explaining each patch\nin the series. From my experience contributing to the Git project, I find it\nto be rare for any patch series to be accepted in the very first iteration.\nFeedback from the community is vital for the refinement of any patch and\nmust be addressed by performing the suggested changes and sending the work\nback to the mailing list. This must be repeated until the work is merged\ninto ‘seen’, ‘next’ and further down, ‘master’.\n\nTimeline\n--------\n\nI’m confident that I can start the project as early as the start of the\nCommunity Bonding Period (May 1 - 26). This is because I have read\nthe related documentation and already have some experience with the idea.\nI believe I’ll be ready to get up to speed to work on the project by then.\nThe exact time arrangement for each test is variable and hard to determine,\nbut judging from the fact that it took me 3-4 days to complete the first\niteration of the t-prio-queue work, here is a proposed migration schedule:\n\nThe first few steps of the plan are easy enough to knock out in a day,\nthe time required to port the tests depends mostly upon the work\nrequired in translating the shell script. As mentioned previously, it\ntook me 3-4 days to complete the first iteration of the test-prio-queue\nmigration patch and that was a short test with only about 50 or so lines\nof shell scripting and all the test cases following a single pattern.\nConsidering all this, I believe it should be possible, on average, to\nmigrate a new test in 4-7 days.\nFrom there, it’s a matter of polishing the patch until integration with\n‘master’ by addressing the feedback on the mailing list which can\ndeceptively take longer than expected. For instance, I had to continue\nrefining my t-prio-queue patch for around 2 weeks after the first\niteration to get it merged to ‘next’.\n\nHence, it should be possible for me to migrate >=8 tests throughout the\nduration of this project.\n\nBlogging\n--------\n\nI plan on writing weekly blogs on the weekends summarizing my work and\noutlining future plans here (link: https://chand-ra.github.io/)\n(I have yet to set up the blog). This is because I believe jotting ideas\ndown help you understand them better while simultaneously serving as a\nguiding path for new contributors to get started with the Git project.\nI learnt quite a lot from previous intern’s blogs like Shaoxuan Yuan’s\nGSoC 2022 Blog (link: https://ffyuanda.github.io/), Shuqi Lang’s GSoC\n2023 blog (link: https://cheskaqiqi.github.io/) and Achu Luma’s Outreachy\n2023 Blog (link: https://gitlab.com/lumap/lumap.gitlab.io) and plan on\nleaving something similar for other newcomers.\n\nAvailability\n------------\n\nMy summertime is reserved for GSoC, so I expect that I can work on a new \ntest 5 days per week, 6-8 hours per day, that is 30-40 hours a week.\nOn the weekends, I would like to solely work on the feedback from\nthe mailing list and advance my [WIP] patches. Certainly, something\nunexpected may arise, but I will try my best to adhere to this time\ncommitment and be always available through the community’s mailing list.\n\nPost GSoC & Closing Remarks\n---------------------------\n\nWhen I first started contributing to the Git project in October of 2023,\nI had no idea about programmes like GSoC. I was advised by a senior of\nmine to contribute to open-source projects and hence, my aim of contribution\nwas to apply what I had learnt in college to solve real-world problems\nand learn from more experienced peers. However, most of what I have\ncontributed to Git has been trivial owing to my lack of skills and\ninexperience with the project.\n\nSeeing how I need to do an internship in summer, with GSoC, I hope to be\nable to dedicate this internship time and effort to a cool project like\nGit while simultaneously learning skills to be able to make more useful\ncontributions in the future. It’s two birds with one stone. I would also\nlike to keep working on this project to see it to completion post-GSoC\nand help mentor other newcomers get started with the Git project.\n\nThanks & Regards,\nChandra\n"},{"id":"491817","messageId":"63059d3b-e883-4fa2-8364-64f7bbd064f7@gmail.com","threadId":"61153","inReplyTo":"ZfwnrL6Zl_lcV09y@tanuki","subject":"Re: [RFC][GSoC] Proposal v2: Move and improve reftable tests in the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-03-29T07:08:31Z","receivedAt":"2024-03-29T07:08:35Z","isPatch":false,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Thanks for the feedback, Christian and Patrick! With your advice, I\ndecided to split my original proposal into two to conform to what was\nsuggested by the SoC 2024 Ideas page.\n\nThis is the proposal for the reftable tests migration project. However,\nI am unsure of what would be a good project size for this project.\nI have quite a long summer vacation and don't really have any other\nplans other than GSoC as of now so I decided to go with large project\nsize on GSoC's website but please let me know if another size would\nbe more appropriate.\n\n---------<8----------<8----------<8----------<8----------<8----------<8\n\nPersonal Info\n-------------\n\nFull name: Chandra Pratap\nPreferred name: Chand\n\nE-mail: chandrapratap3519@gmail.com\nPhone: (+91)77618-24030\n\nEducation: SV National Institute of Technology, Surat, India\nYear: Sophomore (2nd)\nMajor: Mathematics\n\nGitHub: https://github.com/Chand-ra\n\nBefore GSoC\n-----------\n\n-----Synopsis-----\n\nA new unit testing framework was introduced to the Git mailing list last\nyear with the aim of simplifying testing and improving maintainability\nby moving the current testing setup from shell scripts and helper files\nto a framework written wholly in C. The idea was accepted and merged\ninto master on 09/11/2023. The choice of testing framework and the\nrationale behind the choice is thoroughly described in\nDocumentation/technical/unit-tests.txt.\n\nThis project aims to extend that work by moving the reftable tests from\nthe current setup to the new unit testing framework and improving the\ntests themselves. The difficulty for the project should be medium\nand it should take somewhat around 175-350 hours.\n\n-----Contributions-----\n\n• apply.c: make git apply respect core.fileMode settings\n-> Status: merged into master\n-> link: https://public-inbox.org/git/20231226233218.472054-1-gitster@pobox.com/\n-> Merge commit: cf47fb7ec7e183a1a1e521a540862fba3c2a89eb\n\n-> Description: When applying a patch that adds an executable file, git \napply ignores the core.fileMode setting (core.fileMode specifies whether\nthe executable bit on files in the working tree are to be honored or not)\nresulting in false warnings. Fix this by inferring the correct file mode \nfrom the existing index entry when core.filemode is set to false. Add a\ntest case that verifies the change and prevents future regression.\n\n-> Remarks: This was the first patch I worked on as a contributor to Git.\nServed me as an essential intro lesson to the community’s working flow and\ngeneral practices.\n\n• tests: Move t0032-reftable-unittest.sh to the unit testing framework\n-> Status: WIP (GitGitGadget PR)\n-> link: https://github.com/gitgitgadget/git/pull/1698\n\n-> Description: t/t0032-reftable-unittest.sh along with t/helper/test-reftable.c\nunit test Git’s reftable framework. Migrate the test over to the new\nunit testing framework to simplify debugging and reduce test run-time.\n\n-> Remarks: An infant version as of now, I tinkered with this after seeing\nthe project list on Git’s SoC 2024 Ideas page to get an idea of the kind of\nwork that will be involved in this project. Probably the most relevant\npatch out of all the ones mentioned here.\n\n• tests: Move t0009-prio-queue.sh to the unit testing framework\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com/\n-> Merge commit: 107023e1c9f981476c505e73eab319db6534a536\n\n-> Description: t/t0009-prio-queue.sh along with t/helper/test-prio-queue.c\nunit test Git's implementation of a priority queue. Migrate the test\nover to the new unit testing framework to simplify debugging and reduce\ntest run-time.\n\n• write-or-die: make GIT_FLUSH a Boolean environment variable\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1628.v3.git.1704363617842.gitgitgadget@gmail.com/\n-> Merge commit: b3049bbb97c9c0d0292bc9239e976cc661961f39\n\n-> Description: Among Git's environment variables, the ones marked as\n\"Boolean\" accept values like Boolean configuration variables, i.e. \nvalues like 'yes', 'on', 'true' and positive numbers are taken as \"on\"\nand  values like 'no', 'off','false' are taken as \"off\". Make GIT_FLUSH \naccept more values besides '0' and '1' by turning it into a Boolean \nenvironment variable & update the related documentation.\n\n• sideband.c: remove redundant NEEDSWORK tag\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1625.v4.git.1703750460527.gitgitgadget@gmail.com/\n-> Merge commit: 6484eb9a97fe3cd81a2d5711183b93494e6ddefa\n\n-> Description: Replace a misleading NEEDSWORK tag in sideband.c that\nreads ‘replace int with size_t’ with another comment explaining why it\nis fine to use ‘int’ and the replacement isn’t necessary.\n\n• make tig callable from PowerShell/Command Prompt\n-> Status: merged into main (Git for Windows)\n-> link: https://github.com/git-for-windows/MINGW-packages/pull/104\n-> Merge commit: 8eb71eb31c6a1b5d41b253c1ab66d981cc448cb0\n\n-> Description: Tig is a text mode interface for Git that ships with the\nstandard Git for Windows package but isn’t callable from PowerShell/\nCommand Prompt by default. Fix this by updating the relevant Makefiles\nand resource scripts.\n\n• fix broken link on Git for Windows’ GitHub wiki\n-> Status: merged (Git for Windows)\n-> link: https://github.com/git-for-windows/git/wiki/Home/_history\n-> Merge commit: 0df663304bce986da6571cca48b34508d4823a11\n\n-> Remarks: A simple fix for a broken link that I stumbled upon while\nbrowsing Git for Windows’ wiki looking for some help with the patch \nmentioned just before this one.\n\n• t4129: prevent loss of exit codes due to the use of pipes\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1636.git.1704891257544.gitgitgadget@gmail.com/\n-> Merge commit: 1b095626937a858e3a68e9c7d1de11b71117bb42\n\n-> Description: Piping the output of git commands like git-ls-files to\nanother command (grep in this case) in t4129 hides the exit code returned\nby these commands. Prevent this by storing the output of git-ls-files to\na temporary file and then \"grep-ping\" from that file. Replace grep with\ntest_grep as the latter is more verbose when it fails.\n\n• t9146: replace test -d/-f with appropriate test_path_is_* function\n-> Status: merged into master\n-> link: https://public-inbox.org/git/pull.1661.v3.git.1707933048210.gitgitgadget@gmail.com/\n-> Merge commit: 90c0c15e56fa761ae8c4cf5f5fe09b329c0a5dc5\n\n-> Description: The helper functions test_path_is_* provide better debugging\ninformation than test -d/-e/-f.\nReplace tests -d/-e/-f with their respective ‘test_path_is_foo’ calls.\n\n• regex: update relevant files in compat/regex\n-> Status: WIP (GitGitGadget PR)\n-> link: https://github.com/gitgitgadget/git/pull/1641\n\n-> Description: The RegEx code in compat/regex has been vendored from \ngawk and was last updated in 2010. This may lead to performance issues \nlike high CPU usage. Fix this by synchronizing the relevant files in \ncompat/regex with the latest version from GNULib and then replaying any\nchanges we made to gawk’s version on top of the new files.\n\n-> Remarks: When I started working on this patch, I thought it was an\neasy fix but the work turned out to be more involved than I anticipated.\nI had to seek help from the other community members, and we have made\nsome good progress, but there is still a lot of cleaning and changes that\nneed to be done. I haven’t found enough time to commit to this again,\nbut it’s surely something that I want to get done soon.\n\n• commit.c: ensure find_header_mem() doesn't scan beyond given range\n-> Status: Dropped\n\n-> Remarks: This was a patch addressing a NEEDSWORK comment in commit.c.\nIt was dropped because René Scharfe found out a better way to fix the\nissue at hand than my approach or what the NEEDSWORK comment suggested.\n\n\n-----Related Work-----\n\nPrior work regarding tests migrations have been performed by other\ncommunity members and previous interns which form a good guiding path\nfor my own approach. Some previous example work:\n\ni) Port helper/test-ctype.c to unit-tests/t-ctype.c\n-> link: https://lore.kernel.org/git/20240112102743.1440-1-ach.lumap@gmail.com/\n\nii) Port test-sha256.c and test-sha1.c to unit-tests/t-hash.c\n-> link: https://lore.kernel.org/git/20240229054004.3807-2-ach.lumap@gmail.com/\n\niii) Port helper/test-date.c to unit-tests/t-date.c\n-> link: https://lore.kernel.org/git/20240205162506.1835-2-ach.lumap@gmail.com/\n\niv) Port test-strcmp-offset.c to unit-tests/t-strcmp-offset.c\n-> link: https://lore.kernel.org/git/20240310144819.4379-1-ach.lumap@gmail.com/\n\nv) Integrate a simple strbuf unit test with Git's Makefiles\n-> link: https://lore.kernel.org/git/20230517-unit-tests-v2-v2-4-21b5b60f4b32@google.com/\n\nvi) t0080: turn t-basic unit test into a helper\n-> link: https://lore.kernel.org/git/a9f67ed703c8314f0f0507ffb83b503717b846b3.1705443632.git.steadmon@google.com/\n\nIn GSoC\n-------\n\n-----Background for reftable-----\n\nGit’s internals consist of mainly three objects: blobs, tree objects and\ncommit objects. The blobs and tree objects are responsible for storing a\nrepository’s content while the commit objects store information about\ncommits in the repo and are responsible for capturing the repo’s\nhistory. Every one of these objects can be accessed through a unique key\ngenerated by a SHA-256 (previously SHA-1) algorithm. To make life\neasier, instead of remembering the hash key for commit objects, we can\nassign a simple name to them, store these names in a file and use that\nfile whenever we need access to the commits. These names are called\n‘references’ or ‘refs’.\n\nSince a repository can contain a lot of commits and branches and hence,\na lot of refs, Git used packed-refs to save space by storing unused refs\nin a single file. However, this arrangement doesn’t scale well in terms\nof both space and performance. This is where reftable comes in. A\nreftable file is a portable binary file format customized for storing\nreferences. Some objectives of reftable are:\n-  Sorted references enabling advanced scans like binary search.\n-  Near constant time lookup for any single reference.\n-  Efficient enumeration of an entire namespace like refs/tags/\n-  Combined reflog storage with ref storage for small transactions and\n   separate reflog storage for base refs and historical logs.\n-  Near constant time verification if an object name is referred to by at\n   least one reference.\n\n-----Plan-----\n\nThe reftable tests are different from other tests in the test directory\nbecause they perform unit testing with the help of a custom test framework\nrather than the usual ‘helper file + shell script’ combination.\nReftable tests do have a helper file and a shell script invoking the\nhelper file, but rather than performing the tests, this combination is\nused to invoke tests defined in the reftable directory.\nThe reftable directory consists of nine tests:\n\n•  basics test\n•  record test\n•  block test\n•  tree test\n•  pq test\n•  stack test\n•  merged test\n•  refname test\n•  read-write test\n\nEach of these tests is written in C using a custom reftable testing \nframework defined by reftable/test_framework (also written in C). The\nframework has four major features utilized in performing the tests:\n\n•  EXPECT_ERR(c): A function-like macro that takes as input an integer\n‘c’ (generally the return value of a function call), compares it against\n0 and spits an error message if equality doesn’t hold. The error message\nitself contains information about the file where this macro was used,\nthe line in this file where the macro was called and the error code ‘c’\ncausing the error.\n\n•  EXPECT_STREQ(a, b): A function-like macro that takes as input two \nstrings ‘a’ and ‘b’, compares them for equality via strcmp() and throws an\nerror if equality doesn’t hold. The error message thrown contains information\nregarding the file where this macro was invoked, the line in this\nfile where the macro was called and the mismatched strings ‘a’ and ‘b’.\n\n•  EXPECT(c): A function-like macro that takes as input an integer ‘c’\n(generally the result of a Boolean expression like a == b) and throws an\nerror message if c == 0. The error message is similar to EXPECT_ERR(c).\n\n•  RUN_TEST(f): A function-like macro that takes as input the name of a\nfunction ‘f’ (a test function that exercises a part of reftable’s code),\nprints to stdout the message ‘running f’ and then calls the function with f().\n\nOther than these, the framework consists of two additional functions,\nset_test_hash() and strbuf_add_void() which are used  exclusively in the\nstack tests and refname tests respectively.\n\nSince the reftable test framework is written in C like the unit testing\nframework, we can create a direct translation of the features mentioned\nabove using the existing tools in the unit testing framework with the\nfollowing plan:\n\n•  EXPECT_ERR(c): Can be replaced by check(!c) or check_int(c, “==”, 0).\n\n•  EXPECT_STREQ(a, b): Can be replaced by check_str(a, b). \n\n•  EXPECT(c): Can be replaced by check_int(), similar to EXPECT_ERR.\n   E.g. expect(a >= b) --> check_int(a, “>=”, b)\n\n•  RUN_TEST(f): Can be replaced by TEST(f(), “message explaining the test”).\n\nThe information contained in the diagnostic messages of these macros is\nreplicated in the unit testing framework by default. Any additional \ninformation can be displayed using the test_msg() functionality in the\nframework. The additional functions set_test_hash() and strbuf_add_void()\nmay be moved to reftable/stack.c and reftable/refname.c respectively.\n\nThe plan itself is basic and does need improvements, but using this plan,\nI have already created a working albeit primitive copy for two out of the\nnine tests (basics test and tree test) as can be seen here:\n(link: https://github.com/gitgitgadget/git/pull/1698)\n\n-----Improvements-----\n\nAs Patrick Steinhardt mentioned in this mail (link: https://public-inbox.org/git/ZfwnrL6Zl_lcV09y@tanuki/),\napart from the port to the unit testing framework, the reftable tests\ncould use some more refinement. This can be done alongside the test\nmigration efforts and can involve:\n\n• Improving documentation for the tests:\n\nThe reftable tests suffer from a lack of documentation. This might be\nfine for the simpler tests like basics tests and tree tests but for the\nmore complex tests, more documentation means easier on-boarding to the\nreftable sub-project for newer people as well as ease of maintenance.\nThis is something that could be worked on during this project.\n\n• Match coding style with rest of the project:\n\nThere are parts of reftable’s code not in accordance with Git’s coding\nstyle. As a simple example, this snippet in reftable/basics_test.c\n(lines 41-48) doesn’t follow the coding conventions:\n\nif (res < sz) {\n\tfoo;\n\tif (res > 0) {\n\t\tbar;\n\t}\n} else {\n\tbaz;\n}\n\nAnd should be re-written as:\n\nif (res < sz) {\n\tfoo;\n\tif (res > 0)\n\t\tbar;\n}\nelse\n\tbaz;\n\nIt should be well worth it to refactor other parts of the tests to align\nwith Git’s coding standards through this project.\n\n• Increase test coverage:\n\nThere may be parts of reftable’s code that are not currently exercised\nby the existing tests, or edge cases being overlooked by the current\nsetup. This could also be improved upon during this project. As an\nexample, while working on the t-prio-queue migration patch, I discovered\nthat prio-queue’s reverse stack functionality was not tested by the\nexisting setup, which I improved upon by adding a new test case to the\nprio-queue test in the unit testing framework.\n\n• Other than these, working on the tests might uncover bugs or there might\nexist issues within the vicinity of this project that could be worked upon.\n\nTimeline\n--------\n\n-----Pre-GSoC (Until May 1)-----\n\nMy end-semester examinations take place during this period, so it would\nbe difficult for me to allocate time to much else. Nevertheless, I wish\nto keep working on my [WIP] patches and finish reading the reftable\ndocumentation (link: https://git-scm.com/docs/reftable) during this period.\n\n-----Community Bonding (May 1 - May 26)-----\n\nContinue learning in-depth about the reftable sub-project with the help\nof mentors, contribute a few patches related to reftable to familiarize\nmyself with the code. Finish the first iteration of the test migration\nproject using the plan outlined prior.\n\n-----Phase I (May 27 - July 11)-----\n\nPolish the new tests in the unit testing framework using the points\nmentioned above and mentors’ advice. Send the resulting tests to the\nmailing list and work on the feedback received from the community. At\nthe very least, get the ported tests merged to ‘seen’ before the end of\nthis period. Continue learning and experimenting with the rest of\nreftable’s codebase.\n\n-----Phase II (July 12 - Aug 18)-----\n\nContinue working on refining the tests until merge with ‘master’. If\nthat is already within sight, focus on extending the reftable tests and\nimproving documentation. Work on other reftable related tasks in the\nvicinity of this project.\n\n-----Final Week (Aug 19 - Aug 26)-----\n\nFinish up the final touches on any of the work done during the project,\nthis involves ensuring the said work is merged with upstream. Write a\nfinal report on the work accomplished during the project and outline\nfuture goals.\n\nBlogging\n--------\n\nI plan on writing weekly blogs on the weekends summarizing my work and\noutlining future plans here (link: https://chand-ra.github.io/)\n(I have yet to set up the blog). This is because I believe jotting ideas\ndown help you understand them better while simultaneously serving as a\nguiding path for new contributors to get started with the Git project.\nI learnt quite a lot from previous intern’s blogs like Shaoxuan Yuan’s\nGSoC 2022 Blog (link: https://ffyuanda.github.io/), Shuqi Lang’s GSoC\n2023 blog (link: https://cheskaqiqi.github.io/) and Achu Luma’s Outreachy\n2023 Blog (link: https://gitlab.com/lumap/lumap.gitlab.io) and plan on\nleaving something similar for other newcomers.\n\nAvailability\n------------\n\nMy summertime is reserved for GSoC, so I expect that I can work on the\nproject 5 days per week, 6 - 8 hours per day, that is 30 - 40 hours a week.\nCertainly, something unexpected may arise, but I will try my best to\nadhere to this time commitment and be always available through the\ncommunity’s mailing list.\n\nPost GSoC & Closing Remarks\n---------------------------\n\nWhen I first started contributing to the Git project in October of 2023,\nI had no idea about programs like GSoC. I was advised by a senior of\nmine to contribute to open-source projects and hence, my aim of contribution\nwas to apply what I had learnt in college to solve real-world problems\nand learn from more experienced peers. However, most of what I have\ncontributed to Git has been trivial owing to my lack of skills and\ninexperience with the project.\n\nSeeing how I need to do an internship in summer, with GSoC, I hope to be\nable to dedicate this internship time and effort to a cool project like\nGit while simultaneously learning skills to be able to make more useful\ncontributions in the future. It’s two birds with one stone. I would also\nlike to keep contributing to reftable after GSoC and help mentor other\nnewcomers get started with the Git project.\n\nThanks & Regards,\nChandra\n\nReferences\n----------\n\n-  The Pro Git book: https://git-scm.com/book/en/v2\n-  Git’s reftable documentation: https://git-scm.com/docs/reftable\n-  Shaoxuan Yuan’s 2022 GSoC proposal: https://docs.google.com/document/d/1VZq0XGl-wCxECxJamKN9PGVPXaBhszQWV1jaIIvlFjE/edit\n"},{"id":"491836","messageId":"CAOLa=ZTdvYtb6VktU5o7+Erg2_9j29dY9vBsaVEPBzU2-cu18A@mail.gmail.com","threadId":"61153","inReplyTo":"e2d91c9f-9c7a-4127-9bc2-41807765241b@gmail.com","subject":"Re: [RFC][GSoC] Proposal v2: Move more tests to the unit testing framework","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-03-29T17:51:35Z","receivedAt":"2024-03-29T17:51:37Z","isPatch":false,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Chandra Pratap <chandrapratap3519@gmail.com> writes:\n\nHello,\n\n> -----Contributions-----\n\nNit: Not sure how these contributions are sorted, but it would be nice\nto group the git-for-windows patches together.\n\n> In GSoC\n> -------\n>\n> -----Plan-----\n>\n> Tests for Git are defined in the t/ directory and use the combination of\n> a helper file (written in C) and a shell script that invokes the said\n> helper file. I will use my work from the patch ‘tests: Move\n> t0009-prio-queue.sh to the unit testing framework’ to explain the steps\n> involved in the porting of such tests:\n>\n> • Search for a suitable test to port:\n>\n> As Christian Couder mentioned in this mail (link: https://public-inbox.org/git/CAP8UFD22EpdBU8HJqFM+=75EBABOTf5a0q+KsbzLK+XTEGSkPw@mail.gmail.com/),\n> there exists a subset of t/helper worth porting and we need some sort of\n> classification to discern these.\n>\n> All helper files contain a cmd__foo() function which acts as the entry\n> point for that helper tool. For example, the helper/test-prio-queue.c\n> file contained cmd__prio_queue() which served as the entry point for\n> that file. The binary for the helper file is then mapped to a different\n> name by helper/test-tool.c which is used by the ‘*.sh’ files to perform\n> the tests. This name can be discovered by searching for the helper\n> file’s entry point in test-tool.c. Continuing the prior example,\n> “prio-queue” was the name for the helper/test-prio-queue.c binary and\n> t0009-prio-queue.sh invoked it like “prio-queue 1 2 get 3 dump”.\n>\n\nIt is really nice to provide an example, but you're referring to files\nwhich used to exist here, which can be a bit confusing. Would be nice to\ntalk about a new helper file you plan to port and what is the general\nflow you'd follow there.\n\n> •  Create a new C test file in t/unit-tests:\n>\n> After finding a test appropriate for the migration efforts, we create a\n> new ‘*.c’ file in t/unit-tests.  The test file must be named appropriately\n> to reflect the nature of the  tests it is supposed to perform.\n> Most of the times, replacing ‘tXXXX’  with ‘t-‘ and ‘*.sh’ with ‘.c’ in\n> the name of the test script suffices. E.g. t/t0009-prio-queue.sh turns\n> to t/unit-tests/t-prio-queue.c.\n> The new C file must #include “test-lib.h” (to be able to use the unit\n> testing framework) and other necessary headers files.\n>\n> •  Move the code from the helper file:\n>\n> Since the helper files are written in C, this step is mostly a\n> ‘copy-paste then rename’ job. Changes similar to the following also need\n> to be made in the Makefile:\n> -    TEST_BUILTINS_OBJS += test-prio-queue.o\n> +    UNIT_TEST_PROGRAMS += t-prio-queue\n>\n\nThis has a good potential for refactoring these files as they're added\nto the new unit testing library.\n\n> •  Translate the shell script:\n>\n> The trickiest part of the plan, since  different test scripts perform\n> various functions and a direct translation of the scripts to C is not\n> always optimal. Continuing the prior example, t0009-prio-queue.sh used a\n> single pattern for testing, write expected output to a temporary file\n> (named ‘expect’) -> feed input to the ‘prio-queue’ helper tool -> dump its\n> output to another temporary file (named ‘actual’) -> compare the two files\n> (‘actual’ vs ‘expect’).\n>\n> In the first iteration of my prio-queue patch, I worked out a\n> straightforward translation of this pattern in C. I stored the input in\n> a string buffer, passed that buffer to the test function, stored its\n> output in another buffer and then called memcmp() on these two buffers.\n> While this did prove to be a working copy, this work was found to be inadequate\n>\n\nbut why? It is important to state why this is not the best approach.\n\n> The test scripts similarly perform other functions like checking for\n> prerequisites, creating commits, initializing repositories, changing or\n> creating directories and so forth, and custom logic is required in most\n> of the cases of translating these, as seen above.\n>\n> •  Run the resulting test, correct any errors:\n>\n> It is rare for any migrated test to work correctly on the first run.\n> This step involves resolving any compile/runtime errors arising from the\n> test and making sure that at the very minimum, all the test-cases of the\n> original test are replicated in the new work. Improvements upon the original\n> can also be made, for example, the original t0009-prio-queue.sh did not\n> exercise the reverse stack functionality of prio-queue, which I improved\n> upon in unit-tests/t-prio-queue.\n>\n> •  Send the resulting patch to the mailing list, respond to the feedback:\n>\n> This step involves writing a meaningful commit message explaining each patch\n> in the series. From my experience contributing to the Git project, I find it\n> to be rare for any patch series to be accepted in the very first iteration.\n> Feedback from the community is vital for the refinement of any patch and\n> must be addressed by performing the suggested changes and sending the work\n> back to the mailing list. This must be repeated until the work is merged\n> into ‘seen’, ‘next’ and further down, ‘master’.\n>\n> Timeline\n> --------\n>\n> I’m confident that I can start the project as early as the start of the\n> Community Bonding Period (May 1 - 26). This is because I have read\n> the related documentation and already have some experience with the idea.\n> I believe I’ll be ready to get up to speed to work on the project by then.\n> The exact time arrangement for each test is variable and hard to determine,\n> but judging from the fact that it took me 3-4 days to complete the first\n> iteration of the t-prio-queue work, here is a proposed migration schedule:\n>\n\nI think the Community Bonding Period could also be utilized to go over\nthe patches submitted to date to move tests to the unit test framework\nand draw learning's from them.\n\n> The first few steps of the plan are easy enough to knock out in a day,\n> the time required to port the tests depends mostly upon the work\n> required in translating the shell script. As mentioned previously, it\n> took me 3-4 days to complete the first iteration of the test-prio-queue\n> migration patch and that was a short test with only about 50 or so lines\n> of shell scripting and all the test cases following a single pattern.\n> Considering all this, I believe it should be possible, on average, to\n> migrate a new test in 4-7 days.\n> From there, it’s a matter of polishing the patch until integration with\n> ‘master’ by addressing the feedback on the mailing list which can\n> deceptively take longer than expected. For instance, I had to continue\n> refining my t-prio-queue patch for around 2 weeks after the first\n> iteration to get it merged to ‘next’.\n>\n\nThis is an important point, while you can probably have tests being\nconverted fast, it would be nicer to reduce the turn around time overall\nand this could mean you spend more time at the start.\n"},{"id":"492034","messageId":"CAP8UFD26BzjOdJPjcbVy-xGD7nBhFsfSu9CyureCLdHp9eUMZg@mail.gmail.com","threadId":"61153","inReplyTo":"63059d3b-e883-4fa2-8364-64f7bbd064f7@gmail.com","subject":"Re: [RFC][GSoC] Proposal v2: Move and improve reftable tests in the unit testing framework","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2024-04-02T10:01:59Z","receivedAt":"2024-04-02T10:02:14Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Mar 29, 2024 at 8:08 AM Chandra Pratap\n<chandrapratap3519@gmail.com> wrote:\n>\n> Thanks for the feedback, Christian and Patrick! With your advice, I\n> decided to split my original proposal into two to conform to what was\n> suggested by the SoC 2024 Ideas page.\n>\n> This is the proposal for the reftable tests migration project. However,\n> I am unsure of what would be a good project size for this project.\n> I have quite a long summer vacation and don't really have any other\n> plans other than GSoC as of now so I decided to go with large project\n> size on GSoC's website but please let me know if another size would\n> be more appropriate.\n\nOn our Idea page, we have \"Expected Project Size: 175 hours or 350\nhours\" for this project. So it's fine for us if you pick one of those\n(medium or large).\n\n> Before GSoC\n> -----------\n>\n> -----Synopsis-----\n>\n> A new unit testing framework was introduced to the Git mailing list last\n> year with the aim of simplifying testing and improving maintainability\n> by moving the current testing setup from shell scripts and helper files\n> to a framework written wholly in C. The idea was accepted and merged\n> into master on 09/11/2023. The choice of testing framework and the\n> rationale behind the choice is thoroughly described in\n> Documentation/technical/unit-tests.txt.\n>\n> This project aims to extend that work by moving the reftable tests from\n> the current setup to the new unit testing framework and improving the\n> tests themselves. The difficulty for the project should be medium\n> and it should take somewhat around 175-350 hours.\n\nIt's fine to give information about the project that is already in the\nIdea page (or even to just copy from that page), but we are more\ninterested to know how you approach the project, and how you want to\nwork on it. So if you give information that is already in the Idea\npage, please say it clearly, so we can easily skip reading that if we\ndon't have much time.\n\nAs we prefer that you give information specific to your approach, I\nthink it would have been more interesting to say that you decided to\ngo with a \"large\" project size for example.\n\n> In GSoC\n> -------\n>\n> -----Background for reftable-----\n>\n> Git’s internals consist of mainly three objects: blobs, tree objects and\n> commit objects. The blobs and tree objects are responsible for storing a\n> repository’s content while the commit objects store information about\n> commits in the repo and are responsible for capturing the repo’s\n> history.\n\nWhen explaining refs with as much detail, I think tag objects are\nimportant too, as refs often point to them too. On the contrary, blobs\nand tree objects are very rarely pointed to by refs. So it's a bit\nstrange that you don't talk about tag objects, but that you talk about\nblobs and trees which are less important than tag objects in this\ncontext.\n\n> Every one of these objects can be accessed through a unique key\n> generated by a SHA-256 (previously SHA-1) algorithm.\n\nActually both SHA-256 and SHA-1 are supported.\n\n> To make life\n> easier, instead of remembering the hash key for commit objects, we can\n> assign a simple name to them, store these names in a file and use that\n> file whenever we need access to the commits. These names are called\n> ‘references’ or ‘refs’.\n>\n> Since a repository can contain a lot of commits and branches and hence,\n> a lot of refs, Git used packed-refs to save space by storing unused refs\n> in a single file. However, this arrangement doesn’t scale well in terms\n> of both space and performance. This is where reftable comes in. A\n> reftable file is a portable binary file format customized for storing\n> references. Some objectives of reftable are:\n> -  Sorted references enabling advanced scans like binary search.\n> -  Near constant time lookup for any single reference.\n> -  Efficient enumeration of an entire namespace like refs/tags/\n> -  Combined reflog storage with ref storage for small transactions and\n>    separate reflog storage for base refs and historical logs.\n> -  Near constant time verification if an object name is referred to by at\n>    least one reference.\n>\n> -----Plan-----\n>\n> The reftable tests are different from other tests in the test directory\n\nNot sure which \"test directory\" you are talking about. I guess it's \"t/\".\n\n> because they perform unit testing with the help of a custom test framework\n> rather than the usual ‘helper file + shell script’ combination.\n\nThere are actually already unit tests in t/unit-tests which don't need\na shell script.\n\nI think it would have been clearer if this paragraph was started with\nsomething like \"Compared to unit tests that use both a \"t/*.sh\" test\nscript in shell and a test-tool helper in \"t/helper/\", ...\"\n\n> Reftable tests do have a helper file and a shell script invoking the\n> helper file, but rather than performing the tests, this combination is\n> used to invoke tests defined in the reftable directory.\n\nIt looks like you are talking about the \"reftable/*_test.c\" files, but\nI think it would be clearer if you spelled that out.\n\n> The reftable directory consists of nine tests:\n\nI think using \"contains\" instead of \"consists\" would be a bit better,\nas there are other things than the nine tests in the \"reftable/\"\ndirectory.\n\n>\n> •  basics test\n> •  record test\n> •  block test\n> •  tree test\n> •  pq test\n> •  stack test\n> •  merged test\n> •  refname test\n> •  read-write test\n\nYeah, this corresponds to the output of `ls reftable/*_test.c`.\n\n> Each of these tests is written in C using a custom reftable testing\n> framework defined by reftable/test_framework (also written in C).\n\nUsing \"reftable/test_framework.{c,h}\" would be a bit clearer as there\nis no \"reftable/test_framework\" file.\n\n> Since the reftable test framework is written in C like the unit testing\n> framework, we can create a direct translation of the features mentioned\n> above using the existing tools in the unit testing framework with the\n> following plan:\n>\n> •  EXPECT_ERR(c): Can be replaced by check(!c) or check_int(c, “==”, 0).\n>\n> •  EXPECT_STREQ(a, b): Can be replaced by check_str(a, b).\n>\n> •  EXPECT(c): Can be replaced by check_int(), similar to EXPECT_ERR.\n>    E.g. expect(a >= b) --> check_int(a, “>=”, b)\n>\n> •  RUN_TEST(f): Can be replaced by TEST(f(), “message explaining the test”).\n>\n> The information contained in the diagnostic messages of these macros is\n> replicated in the unit testing framework by default. Any additional\n> information can be displayed using the test_msg() functionality in the\n> framework. The additional functions set_test_hash() and strbuf_add_void()\n> may be moved to reftable/stack.c and reftable/refname.c respectively.\n>\n> The plan itself is basic and does need improvements, but using this plan,\n> I have already created a working albeit primitive copy for two out of the\n> nine tests (basics test and tree test) as can be seen here:\n> (link: https://github.com/gitgitgadget/git/pull/1698)\n\nNice, but it looks like \"unit-tests/bin/t-reftable-tree\" sometimes\nfails, perhaps because it's missing a call to test_done().\n\nThe rest of your proposal LGTM.\n\nThanks!\n"}]}