mirror of
https://github.com/initialcommit-com/git-sim.git
synced 2026-04-27 03:25:53 +03:00
[GH-ISSUE #55] Design and create a test suite #38
Labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/git-sim#38
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @abhijithnraj on GitHub (Feb 11, 2023).
Original GitHub issue: https://github.com/initialcommit-com/git-sim/issues/55
Originally assigned to: @abhijithnraj on GitHub.
Hi, I have been following this project for some time since its release. Great Work.
But support for several git features coming in, is there any plans or ideas on testing the features. I would like to work on this.
For Manim based tests, we can have a look at the same way manimCE tests their own code. https://docs.manim.community/en/stable/contributing/testing.html and https://github.com/ManimCommunity/manim/tree/main/tests
Hi, @initialcommit-io , is there any test design you have in mind.?
Some of my ideas:
Once done we can add an actions pipeline to ensure continuous integration.
I am working on this, let me know if there are any existing plans on this.
Once again, Thank You for this wonderful project.
@abhijitnathwani commented on GitHub (Feb 11, 2023):
I was about to create an issue to track this @abhijithnraj. This a good point to start integrating tests NOW. @initialcommit-io even I was skeptical while adding support for the
stashsubcommand that I may break something in some other commands while overriding theGitSimBaseCommandmethods. I think we need to start designing the test plan aggressively now. The problems you caught withTyperintegration could be caught easily with some functional test plans.I didn't know how to test the images that will be generated but this looks promising.
Exactly. This will ensure great test coverage and using tools like
pytestwe shall be in a good form to support this for the long term with new features and guaranteeing support for the already existing features.Python version is already an issue with the new
Typerimplementation which I have discussed here.@initialcommit-io commented on GitHub (Feb 11, 2023):
@abhijithnraj Thank you for creating this issue and for your proposed contributions! Yes, it's clear we need to start on this now, as @abhijitnathwani brought up some breaking changes across Python versions that we had not been testing for.
For context - I didn't have any tests going into the project because I wanted to gauge user interest before spending too much time on it, but now that we know the project has a solid user base, we need to prioritize a robust test suite for the reasons you both mentioned.
@abhijithnraj I don't have any specific test design in mind, but I agree with all of your points, and good find on the Manim test option. Maybe a good place to start is if you want to put together a PR with a simplified version of an initial test suite, that covers a single aspect of the codebase - probably
git-sim logsubcommand since that is the simplest - from all perspectives (1), (2), and (3) that you mentioned.Then we can all discuss the PR and see if it will work for the whole codebase/program.
@initialcommit-io commented on GitHub (Feb 12, 2023):
@abhijitnathwani @abhijithnraj FYI I meant to mention - I will be working on a small tool that can generate dummy Git repos with specific numbers of commits / branch structures / etc. I am thinking we can use this in functional tests to create the structure we need for testing certain commands, like merge/rebase, etc.
@abhijithnraj commented on GitHub (Feb 12, 2023):
@initialcommit-io Yes, that can help with generating test data.
@initialcommit-io commented on GitHub (Feb 12, 2023):
@abhijithnraj Do you have a timeline for an initial version of the test suite that provides coverage for the
git logfeature?@initialcommit-io commented on GitHub (Feb 13, 2023):
@abhijithnraj @abhijitnathwani @paketb0te Here is the first version of a tool I created to generate dummy Git repos:
https://github.com/initialcommit-com/git-dummy
I'm thinking we can use this in our functional tests.
@abhijithnraj commented on GitHub (Feb 13, 2023):
@initialcommit-io I am studying the graphical test cases for manim. I think I can raise the first PR at max 2 days. We will add more testcases once we finalize the framework in the PR. Meanwhile I will checkout git-dummy. currently for testing, I wrote a simple code to generate some commits.
@initialcommit-io commented on GitHub (Feb 15, 2023):
@abhijithnraj Sounds great. FYI I added a
--branches=Noption togit-dummy, so now it can easily generate dummy Git repos with an arbitrary number of branches. This will be good for simulating commands that show multiple branches, like merge, rebase, etc.@initialcommit-io commented on GitHub (Feb 16, 2023):
@abhijithnraj @abhijitnathwani @paketb0te Hi again - last update for a bit on git-dummy, I added flags
--diverge-atto set the commit number at which the branches will diverge frommain.I also added
--merge=x,y,...,nso the user can select which branches to merge back intomainif desired.Its not much, but I think at this point git-dummy has enough so that we can use it for basic functional test coverage, to set up automated scenarios/repos we need for the tests.
I am also planning to add hooks for git-dummy into Git-Sim, with something like a
--generateflag. This will allow users to generate a test repo automatically before running a Git-Sim command.@initialcommit-io commented on GitHub (Feb 21, 2023):
@abhijithnraj Hi again! Just wanted to check in and see if this is still something you were working on?
@abhijithnraj commented on GitHub (Feb 26, 2023):
Hi, Still working on it.
@initialcommit-io commented on GitHub (Mar 3, 2023):
Hi @abhijithnraj @abhijitnathwani @paketb0te ! Hope you all are doing great =)
Just a note I released a significant refactor in git-sim v0.2.6 which overhauls the way it parses the commit history and adds several important new options/flags. Git-Sim now properly traces parent/child relationships, which means it can display more accurate and complex graph structures, with multiple branches and the relationships between them.
See the release notes for more details: https://github.com/initialcommit-com/git-sim/releases/tag/v0.2.6
I figured I'd mention it here since it might affect the test suite, depending on how it was being written.
Let me know if you get the chance to try it out. It's likely there are some bugs since we still don't have the test suite set up yet, but I really wanted to get this released since I feel it's an important milestone for the project, will make the tool much more useful for users, and will make future development smoother.
@abhijitnathwani commented on GitHub (Mar 4, 2023):
Hey @initialcommit-io
These are some great features 👌🏻
I'll try these out over the weekend and let you know if I find something broken.
@ehmatthes commented on GitHub (Jun 12, 2023):
Hi @abhijithnraj, @abhijitnathwani. This is such a cool project! Do either of you have the work you've done public at all? I saw your forks, but don't see any additional tests. I would like to jump in, but don't want to step on your toes if you're about to share some progress.
@abhijitnathwani commented on GitHub (Jun 12, 2023):
Hey @ehmatthes
Thanks for taking the time to check the project. Sadly, I don't have any python testing experience and given the nature of the project we need to design some testing framework for the images it renders. Afaik, we don't have any major progress done on the testing part. We are all open to contributions on this front!
You can represent your thoughts about how we can achieve this and we can review it with @initialcommit-io :)
@ehmatthes commented on GitHub (Jun 12, 2023):
Sure, here's a quick take on how I'd start this.
Because this is still an evolving project, I'd focus on end to end tests and not worry too much about unit tests. I think I can write a demo test suite this week that would do the following:
git-sim statusagainst the test project.pytest handles this kind of setup really nicely. If everything passes, you never even see the temp dir. If the tests fail, you can drop into that temp dir and see what happened. In that temp dir you can then run the command manually, and do a number of other troubleshooting actions.
If that sounds good to you all, I should be able to share a demo by the end of the week.
@initialcommit-io commented on GitHub (Jun 12, 2023):
@ehmatthes This sounds great. I completely agree that end-to-end tests make more sense for the project at this point, to provide baseline insurance that code changes won't break functionality.
I think the tests should catch 2 types of errors caused by code changes (let me know if you have other ideas):
The nice thing about comparing to a reference image is that it is comprehensive - the image captures all elements drawn by git-sim, including commits, arrows, text, etc. So any unintended change to those would cause a test failure. The other benefit is that the default git-sim output is an image file, and this corresponds to the final frame of git-sim video output if the user uses the
--animateflag, so verifying the images should be sufficient to guarantee a good video as well.One downside of comparing images might be performance, so I'm curious how that turns out. Another consideration is that reference files will need to be updated/maintained as functionality changes over time.
For setting up the Git repos to use in each test case, I recommend (again :D) using git-dummy for reproducibility and flexibility. Certain test cases require a Git repo to be in a certain state, and the ability to generate a consistent repo to meet that criteria is why I created git-dummy. Ideally the dummy repos would just be created/cleaned up as a part of the test cases. If you run into a scenario that git-dummy can't currently generate a repo for, let me know and I can add that functionality into git-dummy.
Let us know if you have any other thoughts! Looking forward to seeing what you put together as a demo 😄
@ehmatthes commented on GitHub (Jun 12, 2023):
Yes, this matches my thinking as well.
I have done this in another project, and performance was not a significant issue. For end to end tests you have to generate the images that the project depends on. Once they're generated, the comparison itself does not take long.
One really nice thing about this approach is that it makes it really straightforward to update the reference files. When you run a test, you can pass a flag to stop after the first failed test. You can then drop into the test's temp dir, and look at the generated image that doesn't match the reference file. If this image is correct, you can simply copy that into the reference file folder, and the test will run. You don't have to generate the test file in a separate process; running the tests and failing becomes a way to keep the test suite up to date. The test suite becomes much more than just a pass/fail test, it shows you exactly what the current code would do for an end user.
Does git-dummy generate the same hashes each time it's run? That seems like an issue for testing.
@initialcommit-io commented on GitHub (Jun 12, 2023):
Great point. The hashes would screw that up, since they depend on the timestamp of the commit which would be regenerated each time git-dummy is executed.
Git-Sim actually has a global flag called
--highlight-commit-messageswhich hides the hash values. We could use this, however I just noticed there is a visual bug with that makes the commit message text too big and often overlaps between commits, which would give us ugly reference images. One option is just to fix the overlapping and use the existing flag.Another option is that I can create a new global option for Git-Sim called
--show-hashes, which is true by default. That way in our test scripts we can supply something likegit-sim --show-hashes=false logso that we avoid the non-matching hash issue you mentioned.Alternatively, we might be able to update git-dummy to use hardcoded, expected timestamps via environment variables
GIT_AUTHOR_DATEandGIT_COMMITTER_DATE. These could be set and then unset during each run of Git-Dummy, so that we always get the expected SHA1s on our commits...Do you have any thoughts/preferences?
@ehmatthes commented on GitHub (Jun 12, 2023):
I don't think any of this is an issue. I think it's possible to create a single git repo to use in all tests. There will be new git-sim commands run, but not new git commands run. So git-dummy might help to build the initial test suite, but it doesn't need to be used on each test run.
@initialcommit-io commented on GitHub (Jun 12, 2023):
Hmm, we could do that, but using a single repo to cover all test cases could get increasingly complex as more and more test cases need to be covered. To eventually get full scenario coverage we may need to jump around in the Git repo to do stuff like checkouts/switches/merges/rebases from different starting states. There are also certain scenarios captured by git-sim logic that are based on other factors, like the number of commits in the repo / on the active branch. For example, in a repo with less than 5 commits, there is special logic to handle the display.
My thinking with git-dummy is that it would be used to create a tailor-made repo for each test case with the desired number of commits, branches, merges, HEAD location, etc, and then just clean it up after each test run. This guarantees a minimally complex repo that suits the specific goals of each test case. Of course it does add time since git-dummy would need to repeatedly create/clean up the reference repos, but it performs pretty fast and has the benefit that the git-dummy command for each test case could be hard coded into the test cases, instead of having to ship a test repo around with the codebase.
Maybe there is a happy medium where we can use git-dummy to create a handful of test repos that are each suited to subsets of test cases, instead of generating and cleaning up a new repo for each test case... Thoughts?
@ehmatthes commented on GitHub (Jun 12, 2023):
Is there any way to prevent the image from opening automatically when calling something like
git-sim log?I tried
--quiet, but that just seems to suppress CLI output. I didn't see an obvious place in the code where the generated image is opened; maybe that's something from manim?@ehmatthes commented on GitHub (Jun 12, 2023):
Never mind, I just found it:
git-sim -d log@initialcommit-io commented on GitHub (Jun 12, 2023):
Yes! Use the
-dflag to suppress the image auto-open! Like:git-sim -d logYou can use in conjunction with
--quietto suppress the CLI output as well.@ehmatthes commented on GitHub (Jun 12, 2023):
Okay, I think the test for
git-sim logis working. @initialcommit-io, are you on macOS? If so, I think it will be pretty straightforward for you to run this demo in a little bit.@initialcommit-io commented on GitHub (Jun 12, 2023):
Awesome - I actually switched to pc last year but I do have a mac as well I can try it on... Feel free to submit a PR to the dev branch and I'll take a look.
@ehmatthes commented on GitHub (Jun 12, 2023):
Here's the repo, and the issue tracking my work.
I think you can try this out with the following steps, without dealing with a PR right away:
The most important part in this output isn't really the passing tests. In the first part of the output, look for these lines:
That gives you the absolute path to the sample repo that was used in the tests. You can poke around that folder, and you'll find the
git-sim_mediadirectory. It took a couple hours to set up the whole test suite, but once that was working the second two tests took ten minutes. For example once the first test forgit-sim logran, I copied the test and changed the command that was being tested togit-sim status. That test failed in the final file comparison, but I just popped into the test sample repo and copied the filegit-sim-status_06-12-23_14-26-40.jpgto the reference files, calling itgit-sim_status.jpg.You can also manually run additional git-sim commands in the test repo after the test has finished running:
If this looks good, I'll do some cleanup and submit a PR. The tests aren't super fast, but:
This is on an M1 Mac Studio. One advantage of this approach is it's used exactly as end users work. It's not just running some code from the project, it's issuing the same commands end users issue, and testing the results of those commands. These are the kinds of tests that really let you sleep at night. :)
@ehmatthes commented on GitHub (Jun 12, 2023):
I meant to add that this is fantastic for bug fixing. Here's the process:
@ehmatthes commented on GitHub (Jun 12, 2023):
Last comments until you've had a chance to try it out. Please don't feel any obligation to accept this. I'm a maintainer as well, and sometimes people have taken my work in a direction I don't want to go. I've enjoyed this, and learned some things I wouldn't have if I didn't spend a morning on this project. No hard feelings in the least if you don't want to use this approach!
@initialcommit-io commented on GitHub (Jun 13, 2023):
Hey wow this is really cool, and it definitely moves us many steps closer to having a test suite for git-sim!
And thanks for your last message 😄 - this is my first time maintaining a project that has gotten some real traction, and it can be easy to feel pressured into accepting code changes.
I ran the steps like you mentioned on my Mac and it worked (except the 3 tests seem to have failed due to the files differing, which I confirmed using
vim -d file1 file2in terminal). Didn't dig into why they didn't match though.There are a few design choices (possibly due to my biases of my current workflow 😎) that I prefer:
I'm still not a huge fan of storing and shipping the sample repo with the codebase. I think it feels like an extra piece of baggage that can be generated on the client, so why carry it around? Esp if the current code is taking the time to copy it to a tempdir, which might be comparable to the time of just autogenerating the sample repo(s) the first time the tests are run.
Also not sure how I feel about creating a virtual env as a part of the test suite and installing everything in it. The manim deps are not fun, and my first run took 96 seconds, and subsequent runs took 70 seconds (still takes time to do the venv checks even when the deps are installed). If the user wants a venv they can create one and activate it before running the test suite (which they likely already would have done if developing with a venv), or they can just use a manually-set-up editable git-sim install like
python -m pip install -e .(after navigating into the working codebase). Wouldn't this provide the benefits of the bug fixing scenario you outlined above while saving time of configuring the venvs each time?Would love to hear your thoughts and let me know if I'm missing anything...
@paketb0te commented on GitHub (Jun 13, 2023):
Do you have an idea how we could make the hashes be always the same? Maybe we could create the commit timestamps randomly (not with the current time, but randomly chosen - not sure if that's possible?) and then use a fixed seed for the random module (so it will always output the same for testing) 🤔
Or is there is a way to hardcode the commit hashes directly?
Otherwise I think this is not too bad, since we will not ship the tests and test data with the package, but of course it will be part of the git repo. Also, I assume that the "copying over the test repo"-part is decently fast, the venv setup + installation of dependenies is probably what takes most time.
Agree, I don't really see the advantage of creating a venv from "inside" the tests - when developing, you are (hopefully) already in a venv, most likely an editable install... maybe I am missing something?
But I am glad to see that you took the initiative, thanks @ehmatthes !
@initialcommit-io commented on GitHub (Jun 13, 2023):
See my https://github.com/initialcommit-com/git-sim/issues/55#issuecomment-1587871492 above. (esp the last paragraph)
@ehmatthes commented on GitHub (Jun 13, 2023):
Hi again, and thanks for giving this a try.
That's not good! Tests that pass on my system and fail on yours are not useful!
It turns out that generating jpg or png output can vary slightly across systems. I had hoped that using pngs would generate identical output, but that's not the case. The output images look the same on both systems, but the pixels differ slightly. Image comparison is a huge topic. In the end I replaced the use of
filecmp.cmp()with a customcompare_images()function that compares pixels, and fails the comparison if they differ above a certain threshold. The comparison takes about 0.15s for each test.If you run the updated version, they should pass on your system:
For the development phase of the test suite, I put in some diagnostics. Here's some selected output, on the M1 Mac Studio:
The sample repo is 39kb. I think the biggest reason to include it for now is that we need stable hashes in order to test output. If we generate a repo every time we run tests, we'll have different hashes and the reference images won't match. I absolutely think hashes should be included in the reference images, because most users will include hashes.
In the test setup, most of the time is spent installing git-sim and its dependencies to the tmp environment. Copying the test repo to that environment takes less than 0.02s.
I hear you, it's not fun seeing an initial test suite take on the order of minutes with just three tests. The main reason for this approach is that ideally, test environments should be disposable. That ensures that test runs are consistent, and aren't impacted by leftover state from previous test runs, or inconsistent environments across systems.
I think the alternative to creating a tmp venv on every test run is to create one in a specified location, say
~/git-sim-tests/. If that directory is not found, create it and create the full test environment. If it is found, make sure there's an editable install of git-sim available. You also have to do some work to make sure there's nothing left over from previous tests that will affect the new test run. This can be done, but there's a reason people really don't prefer it. It's easy to think you've reset the environment, and end up with some accumulated cruft that affects new test runs in simple ways. This can lead to passing tests that should fail, and failing tests that should pass. You can make a recommendation that people destroy their test environment and rerun the test suite when tests fail for reasons you can't quickly identify. But then you can find yourself manually doing the destroy/rebuild work that a tmp dir approach handles automatically.People should definitely be able to run tests from the project root, without having to go set up an environment first, and certainly not having to go to an external directory to run the tests. Otherwise you lose the benefit of an automated test suite, and when people report failures you have to ask them about how they are running their tests. Also, people should not try to run tests in an environment where they're running git-sim against any project other than the test repo.
One of the things that slows the test down is that the first time you run a git-sim command in a fresh environment, it takes a long time. On my intel Mac, setup for the test suite takes ~30s, but the first test of a git-sim command takes 36s. Subsequent commands take closer to 10s. That roughly matches what I see if I set up a new git project, install git-sim, and try using it.
If you're curious, I'd be happy to take a look at building out a setup fixture that looks for an existing test environment. Talking this through, it would be interesting to do that, and then add a CLI arg. You could have
pytestrun tests in the stable test environment by default, andpytest --use-tmp-envrun tests in a fresh tmp env.Last thought on this note, some of my hesitation also comes from being reluctant to write to a part of the developer's file system that they might not be expecting. pytest handles temp dirs by tucking them away on a part of the filesystem that's really safe, established, and well-tested. We can't make a directory in the git-sim repository itself for a variety of reasons. There are probably ways to inform the developer of what we're doing, but generating output and prompting for input is not straightforward with pytest, because tests are expected to run non-interactively.
If you were going to build a stable test environment on a developer's system, where would you build that environment? My guess would be
~/git-sim_test_runs/, or maybe alongside wherever they placedgit-sim/.@ehmatthes commented on GitHub (Jun 13, 2023):
One more quick note. You can uncomment this block, and the tests will fail. Those lines run a
git resetcommand, which makes the generated git-sim images differ from the reference files.For example, here's the reference file for testing
git-sim log:With those lines uncommented, here's the file that's generated during the test run:
These images look pretty similar, but they do fail the comparison test. When evaluating the
compare_images()function, it's nice to be able to quickly make a test run that should fail, even though images are successfully generated and look quite similar to what's expected.@ehmatthes commented on GitHub (Jun 13, 2023):
Hi @paketb0te, I may well be unnecessarily carrying over a mindset from a different project to this one, and making the setup more involved than it needs to be. Can you describe your setup for developing this project? I would guess it's something like this:
Is that accurate? Are you then proposing we'd run tests in the git-sim-work-dir/ directory? Or at least use that environment to run tests?
@paketb0te commented on GitHub (Jun 13, 2023):
To be fair it's been a while since I last touched
git-sim😄My usual flow (not only for
git-sim) ispython3 -m venv .venvsource .venv/bin/activatepip install -e .poetry, these three steps are instead a singlepoetry install)-> I use the same virtual environment for developing and testing.
This has been working pretty well for me, but I am always happy to learn about new / improved / better approaches 😃
The git repo on which the tests are run can live wherever, I guess pytests tmpfile stuff is great for that. As long as the virtual environment where git-sim is installed is active, it does not matter where the test-repo is.
@ehmatthes commented on GitHub (Jun 13, 2023):
That's great! This is really helpful. I got in the habit long ago of making my venv in the project where I'm playing around, not in the library itself. I think that's because I did a lot of work in venvs before I started working directly on libraries. I'm also influenced by a project that does require a completely separate environment for testing.
I'll be back with something more efficient. :)
@initialcommit-io commented on GitHub (Jun 13, 2023):
@ehmatthes Sweet! The tests pass now (although I did initially get errors until I manually installed pillow and numpy into the venv).
Wow... I have also assumed till now that the generated image content would match across environments... Good to know that's not the case (that knowledge may save headaches down the line) and nice alternative solution with numpy, even if it may require a bit of tweaking the
ratio_diffover time.I may be nitpicking on this a bit, but it's not so much the size of the sample repo, but the idea of its inclusion (including object database and other git config files like sample hook files) inside the git-sim source code. It just feels a bit sloppy and like something that should be generated if we can. As for getting consistent hashes, I had this suggestion above:
Alternatively, we might be able to update git-dummy to use hardcoded, expected timestamps via environment variables GIT_AUTHOR_DATE and GIT_COMMITTER_DATE. These could be set and then unset during each run of Git-Dummy, so that we always get the expected SHA1s on our commits...
I was just writing up a response about how your thorough points about the venv setup make a lot of sense, but now I'm curious what the more efficient option will be 😅
@ehmatthes commented on GitHub (Jun 13, 2023):
@paketb0te, thank you so much for jumping in. I had never seen
pip install -e .before.The current version of the tests run in 6s on my Studio, and 24s on my intel Mac. They take closer to a minute when first cloning and getting all set up, but after that they run consistently at those two speeds. Most of that time seems to be just the time that git-sim typically takes to run when generating an image.
@initialcommit-io I think you can pull
start_testingagain and you should see that same speedup.I was wary of using Pillow and NumPy, but they were already installed in the environment where I cloned git-sim. I just cloned the project again to verify that. (Clone your git-sim, make new venv, run
pip install -e ., NumPy and Pillow are both present.)I can understand your hesitation to have a sample repo within the project. I would argue that it's not unusual at all to have a number of static resources in a tests/ directory. If it were straightforward to generate a sample repo for each test run I'd be all for that. But overriding something as fundamental as git hashing seems like a lot more work than simply having a small sample repository in the test resources.
That said, I did rename the .git dir to .git_not_a_submodule, otherwise Git thinks the overall project contains a submodule. Are there likely to be other issues as that sample repo grows more complex?
@initialcommit-io commented on GitHub (Jun 13, 2023):
After a clean setup, I got FileNotFoundErrors (tried with and without venv, but maybe doing something silly):
Ahh got it!
I see it less as overriding the hashing and more as customizing a timestamp, but yes let me test that out in git-dummy and see if it's actually as simple as I claim 😄...
@ehmatthes commented on GitHub (Jun 13, 2023):
Here's the block that determines what command to use for running git-sim:
So it's trying to run the command:
When I ran
pip install -e .in the git-sim directory, I got a git-sim command in bin/. Where is your git-sim command?@ehmatthes commented on GitHub (Jun 13, 2023):
I'm not one to talk about what's simple and what's complex today. 🤦
@initialcommit-io commented on GitHub (Jun 13, 2023):
Oops I knew I was doing something silly... I had git-sim installed in every location except the venv in the clone of your fork that I was working in...
Nice, it works now in 45s on the first run and 18s after that (on my Intel macbook pro).
Speaking of the time difference between the first and subsequent runs - this was something I had noticed and has also been reported various times by users. My only thought atm is it may be due to not having the pyc files on the first run, which are then generated and available in future runs? Manim does invoke some programs like ffmpeg for rendering, but I would think calls to that would be constant time. Any other thoughts on that?
@ehmatthes commented on GitHub (Jun 14, 2023):
I have seen that kind of behavior with other libraries as well. Matplotlib definitely comes to mind. I'm not sure there's anything to do about it, though.
If things are heading in the right direction, I can do some cleanup and submit a PR. I'll see that it runs on Windows as well.
@initialcommit-io commented on GitHub (Jun 14, 2023):
Sounds perfect!
@initialcommit-io commented on GitHub (Jun 14, 2023):
Plz submit PR to dev branch
@abhijithnraj commented on GitHub (Jun 14, 2023):
@ehmatthes I am happy to see you have taken over this issue. @initialcommit-io I did not get a lot of time to work on this issue rather than the time I spent initially researching manim graphical unit tests.
I followed manim's own graphical unit tests since they had a really good underlying testing framework for frame comparison both from the final result and frame by frame comparison of the video.
I have updated all that I had to this branch https://github.com/abhijithnraj/git-sim/tree/unit_test. @ehmatthes Please check it if you think it will help. But I see you have already made significant progress.
@abhijithnraj commented on GitHub (Jun 14, 2023):
@initialcommit-io @ehmatthes I saw some discussions above regarding maintaining the same commit hashes. I faced the same problem while I checked with git dummy. That's why I created https://github.com/abhijithnraj/git-sim/blob/unit_test/test/temp_git_creator.py to prevent this blocker until such changes could be introduced to git_dummy. Feel free to use any of the code changes in the branch.
@initialcommit-io commented on GitHub (Jun 14, 2023):
@ehmatthes Thanks for the PR and for adding that documentation!! Overall looks excellent.
I ended up having some extra time tonight so I added a new flag to git-dummy called
--constant-shawhich keeps the commit SHA's consistent by hardcoding the commit author, email, and commit date into all commits.I released this as git-dummy version 0.0.8. Since git-dummy is a dependency of git-sim (and is installed along with it) all users using pip to install git-sim should get 0.0.8 now.
Can you update the PR to replace the sample repo with a call (in the code) to git-dummy to autogenerate one using the
--constant-shaflag? I think you can use the same git-dummy command you used to create the sample repo but just add that flag. For now we can just use a single repo call to cover all test cases, and later on we can add cleanup and/or additional calls to create new repo structures as needed.I know this involves some code changes, documentation changes that reference the sample repo, and the reference images will need to be redone with the new constant-sha repo... Sorry for the late change-up, I didn't think I'd have time to get git-dummy updated this soon.
@ehmatthes commented on GitHub (Jun 14, 2023):
No problem! "92 files changed" was a nice motivation to land that flag, right? :)
@ehmatthes commented on GitHub (Jun 14, 2023):
@abhijithnraj I started looking at Manim's testing approach, but it was looking more complex than I could make use of in a reasonable timeframe. There may well be a better approach to verifying the output of test runs, but that should be somewhat straightforward to implement when the time is right. I'm a big fan of "good enough" when getting an initial test suite off the ground. I also haven't touched testing for animation output at this point.
For the record, pytest makes it really straightforward to choose which tests you want to run. I'm assuming the video tests would be a lot slower than the image tests. You can run the animation tests less often than the image tests. As the test suite grows, you can choose which tests to run at various stages of the development cycle.
@initialcommit-io commented on GitHub (Jun 14, 2023):
LOL 😜 and thank u!
@initialcommit-io commented on GitHub (Jun 14, 2023):
Agree. Not sure we even need animation testing at any point since the image file output should be identical to the final video frame. It's not 100% foolproof since theoretically you can get to the same final state through multiple paths, but is likely be good enough esp when considering much longer it would take to generate and compare the animated video files.
@ehmatthes commented on GitHub (Jun 14, 2023):
Okay, I'm really skeptical of trying to generate the sample repo on every test run. I just made the changes to use git-dummy during each run, on the use_gitdummy branch in my fork. (This is a branch off of the
start_e2e_testingbranch.)I ran the test suite once, and all three tests failed as expected. All of the images looked correct, so I copied those over as reference files. Then I ran the test suite three more times, and each time one test would fail. Here's the command that's using git-dummy to generate the sample repo:
This creates an interesting repo, which is good for testing. But I think there are two non-deterministic aspects. One, I believe the merge that happens is random each time. Two, and maybe related to that, it is generating different hashes at times.
I'm skeptical because as you start to model more complex interactions I think you're going to see more issues like this pop up.
git-dummyis an interesting and useful idea, but I'm not sure it's worth trying to keep that project as robust as it needs to be to support this project. Then again, I'm not in the weeds in git-dummy, so maybe there's a straightforward way to address this. Maybe a no-random flag or something?I'm not in any rush, and this only affects the tmp_repo() fixture in contest.py. It's not difficult to trade out approaches for a while longer.
@ehmatthes commented on GitHub (Jun 14, 2023):
I meant to add you should be able to try this out by:
git pull origin use_gitdummypytestthree times, without the-xflag; you want it to generate all three image files on each test run.@ehmatthes commented on GitHub (Jun 14, 2023):
I tried removing
--merge=from the git-dummy call, but that still didn't fix things. Maybe --constant-sha is not being used when creating branches?I have to step away from this for most of the day, but if I'm quiet I'll be back tonight or tomorrow.
@initialcommit-io commented on GitHub (Jun 14, 2023):
Ah shoot, I think there is a random element to where the branches diverge at. BUT, this can be set using existing git-dummy flag called
--diverge-at=X, where X is the index of the commit on the base branch to base the new branches off of.Adding it to the end your command:
cmd = f"{git_dummy_path} --commits=10 --branches=4 --merge=1 --constant-sha --name=sample_repo --diverge-at=2"This should make all the branches diverge from the main branch at the 2nd commit. But you can choose X to be what works best i.e. gives the repo the characteristics you want.
Merges (when using the
--merge=Y,Zoption) should always happen at the final commit branch(es), so I don't think that should affect any of the SHAs.Hopefully that should do it.
@ehmatthes commented on GitHub (Jun 14, 2023):
That does seem to fix it on my end. I'll try this on Windows later, and do a little more cleanup, and then update the PR.
Are there any changes you would make to that git-dummy command, to facilitate any further testing you anticipate?
@initialcommit-io commented on GitHub (Jun 14, 2023):
Let's see... I think what you have now (the one above) should be good enough to start. With that we should be able to simulate the following commands:
Commands that will likely need to wait / might need special accommodations:
@ehmatthes commented on GitHub (Jun 14, 2023):
But this is part of the problem with the current approach. When you modify the command, can you reliably do that without affecting any of the previous output? I'm anticipating someone having to regenerate all of the reference files every time there's a change made to the git-dummy call.
@initialcommit-io commented on GitHub (Jun 14, 2023):
That is why my initial thought was to have a git-dummy call specific to each test case, that generates a minimum viable repo needed for that case without impacting any others, and have the repo cleaned up after each test case. Since generating these test repos is a lightweight operation it should perform ok. But I think that's a point we can get to incrementally.
To kick things off I think we can stick to what we have and use a single repo like we've been discussing. This will enable us all to play with the workflow and see how it feels. Then as we create more specific test cases, especially ones that cover sub-scenarios within each command (for example regular merge vs fast-forward merge, checkouts/switches to diverged vs same-line branches, etc) we can break out those test cases with their own customized git-dummy repos as needed.
@initialcommit-io commented on GitHub (Jun 14, 2023):
If need be we can also come up with a programmatic way to have subsets of test cases all use a single repo, to avoid regenerating the same repo over and over, but again this a problem for a bit down the line.
@initialcommit-io commented on GitHub (Jun 14, 2023):
Last thing - wouldn't a static sample repo have the same problem? If it needed to be changed in some way to accommodate a new scenario/test case, we'd have to regenerate all the assets as well.
@ehmatthes commented on GitHub (Jun 14, 2023):
That's pretty interesting. There's a very clear benefit of not having an extra 100+ file sample repo in the test resources.
I'm still a little hesitant about it though, because I could see this splintering into groups of tests that are more driven by issues with git-dummy than how the tests should be grouped for the purposes of testing git-sim.
None of these are dealbreakers though, and I think you'll be quite fine going in either direction. You could also write a script that runs the test suite, expects a whole bunch of failures, and then rewrites the output files as the new reference files. But as I write that out, that sounds like a terrible idea! Tests really need to be stable to be reliable. :/
Oh, I don't know! My first thought was you just execute new git commands in that repo, thus not changing any of the existing hashes. But maybe you do a
git reset --hard old_hash_numbefore some test calls? Copying the static repo is quick, and modifying state is not hard as the test run progresses.I think this all points to the fact that testing a tool that acts on a git repository is going to get complex at some point.
So, at this point I'll plan to clean up the git-dummy based approach and then update the PR. Does that sound good?
@initialcommit-io commented on GitHub (Jun 14, 2023):
Took a quick read through but am on the go and need to look closer later and think about it more.But yes for now let’s go for the git-dummy single repo approach and take it from there.On Jun 14, 2023, at 12:49 PM, Eric Matthes @.***> wrote:
That is why my initial thought was to have a git-dummy call specific to each test case, that generates a minimum viable repo needed for that case without impacting any others, and have the repo cleaned up after each test case. Since generating these test repos is a lightweight operation it should perform ok. But I think that's a point we can get to incrementally.
That's pretty interesting. There's a very clear benefit of not having an extra 100+ file sample repo in the test resources.
I'm still a little hesitant about it though, because I could see this splintering into groups of tests that are more driven by issues with git-dummy than how the tests should be grouped for the purposes of testing git-sim.
None of these are dealbreakers though, and I think you'll be quite fine going in either direction. You could also write a script that runs the test suite, expects a whole bunch of failures, and then rewrites the output files as the new reference files. But as I write that out, that sounds like a terrible idea! Tests really need to be stable to be reliable. :/
Last thing - wouldn't a static sample repo have the same problem? If it needed to be changed in some way to accommodate a new scenario/test case, we'd have to regenerate all the assets as well.
Oh, I don't know! My first thought was you just execute new git commands in that repo, thus not changing any of the existing hashes. But maybe you do a git reset --hard old_hash_num before some test calls? Copying the static repo is quick, and modifying state is not hard as the test run progresses.
I think this all points to the fact that testing a tool that acts on a git repository is going to get complex at some point.
So, at this point I'll plan to clean up the git-dummy based approach and then update the PR. Does that sound good?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>
@initialcommit-io commented on GitHub (Jun 15, 2023):
@ehmatthes Hey sorry about earlier I tried responding via email while in an Uber and the result was kinda... ugly.
I'm less worried about this because the whole point of git-dummy is to be tailored to creating exactly the types of repos we want for git-sim. git-dummy can be molded into exactly what we need for git-sim, not vice-versa.
After chewing on that for the day I feel that if we were talking about unit tests (or even functional tests for more specific cases), then the stability really matters a lot more than for broad end-to-end tests that we're implementing. A huge blind spot in my thought process so far is that if we make a fairly common type of code change to git-sim that impacts all image outputs - say adding a logo into the corner of the images, or simply changing a font, or changing the spacing between commit circles and arrows - all the reference images will need to be recreated for all test cases. Therefore it's sort of implied that we're not doing this end-to-end testing for stability-of-test-outputs over time as git-sim changes. We're doing it so that in the course of everyday development of specific features/bugfixes, we can easily verify that something else didn't get broken. But our end-to-end tests won't help at all if a global change is made since that will likely just break everything with the only viable option being a total overhaul of all reference images. So circling back to your quoted statement above, I think it could be useful to have a script like you suggested that intentionally rewrites all outputs as new reference files, since this would be a fairly common operation when git-sim is altered in a global way.
At that point you're not only adding hardcoded git commands external to git-dummy when they may as well be captured in git-dummy itself, but you're also hardcoding references to specific git states (like resetting to specific commit shas) which isn't as flexible as being able to deal with arbitrarily generated repos. If the sample repo changes down the line, then all those git reset commands need to be updated too.
Hahaha yes this.
That would be wonderful.
@ehmatthes commented on GitHub (Jun 15, 2023):
No problem on the earlier reply. I've been responding quickly because I've had time this week, but I have no expectation that people reply immediately in open source work.
These are all good points. I jumped into this because Git is often seen as more difficult and confusing than it needs to be. This is a great tool for helping people see how Git works, and the fact that it acts on their own repository is fantastic. That's way more impactful than diagrams about a generic sample project. I'm also looking for real-world ways to get better at testing. I really find it satisfying to write the first tests in a project.
One of my testing principles is to not blindly pursue coverage. There are many projects where covering the most critical pathways and outputs is more beneficial to the overall project than trying to build out a test suite that covers all edge cases, but then becomes difficult to use and maintain. All the issues we've been discussing can be addressed quite reasonably with a test suite that has just enough coverage.
That's a really good point about image output changing over time. It should change! I'm also doing this work to get better at using pytest. You can add a CLI argument to pytest that rewrites the reference files. So that work wouldn't have to be separate from the test suite; you could run
pytestand make sure everything fails as expected, and then runpytest --rewrite-ref-imagesor something like that, and it would rewrite all the reference image files for the current OS.I updated the PR. Thank you so much for the discussion, I've learned a lot from this work!
@initialcommit-io commented on GitHub (Jun 16, 2023):
Thank you! I'm honestly so happy that git-sim is (hopefully) helping folks. Took me years to really get comfortable with git and there are still plenty of commands/workflows I haven't used much if at all. And testing is an area I've barely scratched the surface on so your input and work are a huge help. Not sure when I would have gotten this going otherwise :D. Please let me know if you include git-sim in that article/newsletter I'd like to check that out and pass it along through my "initial commit" channels.
Great point. I'm excited to start finding out what that balance is for git-sim. I anticipate it will be something like one test case per "type of git operation that is worth discussing", like what I mentioned above regarding various types of merges, resets, checkouts, etc. These tend to correspond close to 1-1 with logic paths in the git-sim codebase.
Yes, this sounds perfect. I may ping you at some point if I have questions or run into issues with it :D
Seriously, thanks again! I learned a lot too and will try to go through the PR soon and play around with it before (hopefully) merging. Will reach back if any notable changes need to be made.
@ehmatthes commented on GitHub (Jun 16, 2023):
I've been using Git for 15+ years, but I've gotten by that whole time almost entirely using a small subset of commands: init, add, commit, branch, merge, pull. Sometimes I fetch! But I've never rebased. I'm comfortable sorting out whatever I need to do though, because I have a fairly good understanding of how Git works internally. One of the reasons I really like this project is because it helps build an accurate understanding for people of how Git actually works, while using a project they're already familiar with (their own).
The article I mentioned will go out next Thursday, so I'll share a link here when it's out.
This makes perfect sense, and it's what I was expecting you to aim for. I will be curious to hear later on if the test suite has caught any issues before making a release. It certainly should at some point!
There's one other set of tests I think you should consider sooner rather than later. I think it would be really worthwhile to write one test for each of the global options. You might pick a single git operation, and run that one operation with each global setting once.
If you want some ongoing help, I'm happy to keep at this for a bit, at a slower pace than I jumped in this week. pytest has some really nice ways to do this efficiently. Parametrization of tests is amazing. Here's how you could write one test, that would actually be run once for every global option:
The
compare_images()function would be updated to compare the generated image against the correct reference file.If any of those fail, the output tells you exactly which global option/s resulted in the error.
Great work on this project, and let me know if you'd like more help with the test suite.
@ehmatthes commented on GitHub (Jun 22, 2023):
Hi again @initialcommit-io. The article that led me here is posted now, if you're curious to skim it.
@initialcommit-io commented on GitHub (Jun 22, 2023):
@ehmatthes That's awesome!! Thanks for including git-sim!!!!! 😸
@initialcommit-io commented on GitHub (Jul 4, 2023):
Hey @ehmatthes,
So I just tested this out in Windows and am getting permissions errors in the
compare_images()method inutils.py:I tried printing out the paths to the reference files and generated files in the
test_log()method, and the reference file path seems to be correct but the generated file path looks like it's grabbing the current directory as..Did you run into any permissions issues in your Windows testing?
@initialcommit-io commented on GitHub (Jul 4, 2023):
Meant to mention I tested running gitbash as administrator but still got the permissions errors.
@ehmatthes commented on GitHub (Jul 5, 2023):
I did not run into any file permission issues during development, and I don't see any now.
I just modified
test_log()to show the path to the generated file:Here's the output of running just this test on macOS:
And here's the same output on my Windows VM:
I get similar output in a Git Bash shell on my Windows VM. What output do you get for the path to the generated image file?
The path to the generated file is being read from the output of the
git-sim logcommand. You can cd into the sample repo generated by the test suite and rungit-sim logthere manually. (Make sure you are in an active venv, wheregit-simis available):The first command here shows that
git-sim logruns in the repo generated by the test. The second command shows that the full command run by the test suite,git-sim -d --output-only-path --img-format=png log, runs as well.I would be curious to know if git-sim is generating different output in your environment, or if the test code is not parsing the output properly in your environment. If
fp_generatedis not a file path, I'd add one more line totest_log():@initialcommit-io commented on GitHub (Jul 5, 2023):
Sorry about that. It looks like the changes I was making to git-sim and trying to test caused an error that led to the output not being generated at all, hence the confusion.
After tweaking my change, the tests now run, but still fail due to bad ratio diff. I did confirm it is using the Windows reference files to compare, but looks like the fonts used differ between my output and the Windows reference files. I think this happened to you at one point as well, but we thought it was across OS's. This font difference makes the ratio_diff up to 5%.
Attaching images of the git-sim log Windows reference file and my output showing the differing fonts (see below). On my system it uses Monospace Bold which is the font that git-sim is configured to use for all output. I assume the reference files were generated on a system where Monospace was not available as a system font (or maybe just not in the correct bold weight), so it used the default system font as a fallback.
This could get hairy as devs try to test across different systems... I wonder if we could get around this by adding a global
fontargument to git-sim, which we can specifically set in tests to a default font that we know exists in each respective environment.@ehmatthes commented on GitHub (Jul 5, 2023):
Okay, that's helpful. I made a note to add a line to
compare_images()to check that the image file exists, before actually doing the comparison. We can make a similar assertion about the output of each call, so in the future you'll know more quickly whether there was an issue with the output message or the actual image file.This was really helpful to know as well. I think there's a straightforward solution that makes testing much easier across systems. I changed all the instances of "Monospace" in
git_sim_base_command.pyto "Courier New". Here's the output on macOS:Here's the output on Windows:
I believe this works better because "Monospace" just causes Manim to use the system's default monospace font. "Courier New" is a more specific font, which happens to be available on most macOS and Windows Systems. I would be curious to see what the
git-sim logoutput looks like on your system if you change Monospace to Courier New.@ehmatthes commented on GitHub (Jul 5, 2023):
I think the most reliable fix for this would be to bundle a specific font with the test suite. I added a .ttf file to the test suite, and modified
construct()in log.py to use the bundled font. This is a more complex change than just making the font a setting, because from my brief reading Manim expects bundled fonts to be used in a context manager.To experiment with different fonts in the
git-sim logoutput, I changed all references to "Monospace" toself.font, and modified__init__()inGitSimBaseCommand:With this change, all tests still pass. If I set
self.font = "Courier New", tests fail but I get the output shown above.To use a bundled font, I had to make the following change to
construct()inLog:This made fairly ugly output, but I believe the output would be identical on all systems:
I don't think I'd bother using a tiny font like this. I was experimenting with a tiny font, and that's one that showed up in a quick search. Also, I'm guessing you wouldn't have to add a context manager to every command's
construct()method. I imagine with a better understanding of the codebase there's one place to put that context manager that all files can use.@initialcommit-io commented on GitHub (Jul 5, 2023):
Thanks for all the deets! Yes using Courier New in
git_sim_base_command.pymakes the tests work.The quickest and least invasive solution for now seems to be adding a global flag for the user to specify the font, and then hardcoding that as Courier New from the tests raw command.
This will have the added benefit of letting the user customize the font. I will try this out and update here.
And oh yes - this would be great!
@initialcommit-io commented on GitHub (Jul 5, 2023):
Ok - this is implemented on the dev branch. I added a
--fontglobal option which is now set by the test suite. I also updated the reference files for Macos since the test suite on Mac and Windows now uses Courier New font.Wanna try it out and lemme know if it works for you?
@ehmatthes commented on GitHub (Jul 5, 2023):
Okay, the dev branch works for me. I did have to rebuild my virtual environment. That wasn't obvious until I tried to run a git-sim command manually.
I removed the Windows-specific files, and the code that modified the reference file path to look for a Windows-specific file.
I also tried using a bundled font at the test-only level:
This didn't work, and I wasn't too suprised by that. I believe
subprocess.run()opens a new shell, which doesn't see the changes thatregister_font()made to the environment. That said, I think there's a way to make this work if we end up running into the same environment-specific testing issues. I think you could make the test font .ttf file available in a shell, and then make the test's git-sim calls run in that same shell. If you run into an issue with different people generating slightly different but correct images, please ping me and I'll be happy to try building this out.@ehmatthes commented on GitHub (Jul 5, 2023):
I made a PR so you can pull these small improvements if you want. Feel free to reject the PR and incorporate those changes whenever appropriate.
@initialcommit-io commented on GitHub (Jul 6, 2023):
Sure I'll merge the PR, but can you make it to the dev branch instead of main? Looks like it's including 2 of the commits I made since those don't exist on main yet... But those should go away if you change it to the dev branch.
@ehmatthes commented on GitHub (Jul 6, 2023):
I'm newer to committing to others' projects, so some of my Git habits are unclear. I forgot to run Black before submitting the PR, so now the PR would include three commits:
Should I submit a PR with these 3 commits? Do you want me to squash them? Do you squash them when you merge?
@initialcommit-io commented on GitHub (Jul 6, 2023):
Thanks for running Black - this is my first time using it as suggested by other contributors. On my Mac I was able to get Black to to run automatically when exiting Vim (via a Vim plugin), but had issues setting that up on my current Windows machine so now it's manual again, so I often forget to run that myself too 😸.
You can just submit the PR to the dev branch with the 3 commits, I'll squash and merge them in the GitHub interface.
@ehmatthes commented on GitHub (Jul 6, 2023):
Sure thing, I'm trying to use Black more consistently in my own work as well. I resubmitted the PR.
@initialcommit-io commented on GitHub (Jul 6, 2023):
Merged it and added tests for the remaining non-networked subcommands. I regenerated all the reference images on my Windows machine and then tested on my Mac, where 12/18 tests passed and the other 6 failed due to bad ratio diff. Most of these were barely above the threshold, but the biggest difference was about 0.0126.
I'm thinking this is just due to how the image files are generated on different platforms (regardless of font). Anyway, I'm thinking we can just boost the acceptable ratio diff from 0.0075 to 0.015 to take into account these subtle differences.
@initialcommit-io commented on GitHub (Jul 6, 2023):
(the updated reference files are pushed up in the dev branch if you want to test on both your systems to compare)
@ehmatthes commented on GitHub (Jul 6, 2023):
I got the same results on my Mac. The highest ratio came from the log command, but I opened the generated image and the reference image and couldn't see any difference at all. I think a ratio of 0.015 sounds perfectly reasonable. I'm going to be a little embarrassed, but mostly curious if someone who's more familiar with working with images comes along and has a much simpler way to test image output.
The test suite is slower now, as expected when running 6 times as many tests. It takes 80s to run the test suite on my system. I was tempted to focus on the
compare_images()function, but then I remembered to profile. I added profiling code to thecompare_images()function, and around each git-sim call. It takes less than 0.1s to runcompare_images(), but about 3-6s to run each git-sim command:If you want to profile the current suite on your system, it's on my profile_tests branch.
There are a couple ways to speed up tests...
@ehmatthes commented on GitHub (Jul 6, 2023):
Parallel tests
The main way to speed up tests is to run them in parallel. pytest has a really good plugin called pytest-xdist which handles all of the overhead of distributing tests for many test suites. It works for our suite. Install
pytest-xdistfrom PyPI, and then runpytest -n auto. It will figure out the appropriate amount of distribution to do on your system. On my system, it reduces the total test suite time from 80s to 20s.Non-parallel output:
Parallel output:
The
-sflag doesn't work when running tests in parallel, so if you're examining output it's better not to run the tests in parallel. Also, the benefits of running parallel tests are highly dependent on the system architecture. Some people will see more benefit than others from running parallel tests; some people may see worse performance.@ehmatthes commented on GitHub (Jul 6, 2023):
Running selected tests
You can organize the test suite into multiple files. For example you could pick a set of core commands you always want to test, and put those in test_core_commands.py. Then other tests go in test_noncore_commands.py. You'd run
pytest tests/e2e_tests/test_core_commands.pymost of the time, and thenpytestorpytest -n autowhen you want to run all the tests. Of course it doesn't need to be core and noncore, it could be simple and complex, or however you think about the overall set of commands.pytest has a way of marking tests, so you could just mark some tests as core tests. Add the decorator
@pytest.mark.core, and then runpytest -m core. This would only run the tests you've marked as core tests.@initialcommit-io commented on GitHub (Jul 6, 2023):
Sweet! I'll bump that up.
Uhhh..... this is awesome!!! On my PC this speeds things up from 45s to 14s. I think it's a good compromise to gain that kind of speedup by running in parallel, and if something fails to rerun in series to get the output.
Random note on parallelism - I spent a lot of time using the multiprocessing module to try to get git-sim to render individual animations in parallel (by rendering independent segments of each video in parallel) to speed up performance, as I feel that Manim's performance is a major limitation of git-sim.
Unfortunately I couldn't get it working in the end as I had issues passing data (specifically file handles/references) in between processes. It was pretty weird behavior and I was kinda surprised I couldn't get it working - seemed so close. Maybe one day as Python's multiprocessing and pickling tools get more robust...
Good to know!
@ehmatthes commented on GitHub (Jul 6, 2023):
I have one more thought at the moment, not about performance. You can set custom messages to display when a test fails. For example you're seeing "bad pixel ratio: " at times in the output, but if there are multiple test failures you kind of have to hunt for which tests had which ratios. You can write code like this:
Then you'd see that information for failing tests even without using the
-sflag.A lot of the decisions like this just come down to how you want to use your test suite. What kind of information do you want to see in all test runs? What kind of information do you want to see only when looking at results in detail?
@initialcommit-io commented on GitHub (Jul 6, 2023):
Funny you mention that - as a part of the threshold update I just committed I updated that very same printed message to specify the subcommand that failed:
print(f"bad pixel ratio ({path_ref.stem[8:]}): {ratio_diff}")😸
@ehmatthes commented on GitHub (Jul 6, 2023):
Do you mind if I ask about the specs on your system? I think I have a reasonably well-specced system, but your tests are twice as fast without parallel, and still significantly faster when run in parallel. I wonder if there's anything about my setup that's slowing things down for me.
I've got other things going on for a while so I'm going to stick to a focus on testing for the time being, but I would love to look into this at some point.
You might give the assert message a try, because I think this is the only remaining reason to use the
-sflag.@initialcommit-io commented on GitHub (Jul 6, 2023):
I upgraded my PC's CPU to the 13600k late last year, runs a lot faster on my PC now than on my Intel mac.
Oh definitely - I'm grateful for just the testing help this has been incredibly good for the project. Happy to share what I've done on the performance front at some point if your get some extra time!
Oh gotcha I'll take a look - I missed that assert statement in your note.
@ehmatthes commented on GitHub (Jul 6, 2023):
To be clear, that would move the assertion from the individual test functions to the compare_images() function. That seems like a slightly better structure anyway.
I'll go quiet for a while, but if you ever reply somewhere here and don't hear from me, feel free to reach out directly over email. I think I'm going to write a newsletter series on Git at some point, and I'll definitely use git-sim for the visualizations for that series.
@ehmatthes commented on GitHub (Jul 6, 2023):
One more quick thing, you might want to remove the stub file test.py from the top level. It's not causing any problems, but with pytest in use it's just taking up space and it might confuse someone looking at the project.
@initialcommit-io commented on GitHub (Jul 6, 2023):
That's awesome. Please drop me a note when it's ready would love to take a look.
Oh yeah - I thought about that but decided to leave it as a stub for unit tests. But you're right now that we have the
tests/folder it probably at least makes sense to move it into there...@initialcommit-io commented on GitHub (Jul 17, 2023):
Closing this since related work to refine the test suite will be handled thru #99