-
Notifications
You must be signed in to change notification settings - Fork 672
External repos zipped #1309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
External repos zipped #1309
Conversation
This hello-world.git repo is created using the native Git for Windows software. Various test classes in the GitBlitSuite test suite require the presence of the hello-world.git repo in github.com/git/ which has been missing, hence causing many test failures in the suite. This recreation of the hello-world.git repo aims to conform to the many test cases' requirements in the suite, and to be checked in as part of the gitblit repo, thus eliminates the requirement of a remote hello-world.git repo during the test run. The repo is now stored in the new src/test/data folder. The hello-world repo's various commit IDs were hard-coded in various test classes. These commit IDs, which must now have new values in the recreated repo, are now extracted out to the src/test/data/hello-world.properties file. The gitblit's build.xml is modified to generate the HelloworldKeys.java file containing the hello-world.properties file's key strings, in similar fashion as the existing generation of the com.gitblit.Keys.java file. And these key strings in HelloworldKeys.java are now used in the various test classes, thus eliminating the hard-coding of the hello-world repo's commit IDs in the test code.
Most of failures were due to temporary test repos, users and/or teams being left behind after the test run, and these left-over stuff in $baseFolder/data/git caused assertion errors in many tests in subsequent test runs. This fix tries to delete those left-over stuff at the end of each test, mainly in their @afterclass code blocks. PushLogTest.java is deleted as it doesn't work, and has been superseded with better tests in various protocol test suites (GitServletTest, GitDaemonTest, SshDaemonTest, etc.)
During the test run by GitBlitSuite test suite, some repos from GitHub were cloned and became part of the test data. These repos are now zipped to be part of gitblit repo itself, thus eliminating the network fetch at the start of test run which can be slow, especially with the JGit repo cloning which is huge and time consuming. The cloned JGit repo is now zipped and checked in to gitblit, along with the other 4 repos (hello-world, ambition, gitective and ticgit). They will be unzipped during the test suite run and be available in the local file system, thus avoiding the need for some network fetch. Special note on the zipped JGit repo: this repo is big (and growing all the time on GitHub), and takes up about 32MB of disk space after cloning from GitHub. I've made it smaller by resetting HEAD back to a commit of 5 years ago (with git reset --hard <commitId> command), to put it back to roughly where/when the tests were written for it (which is not quite, because there are tons of commit history since which can't be removed.) This local JGit repo is then garbage-collected (with git gc --prune --aggressive) to reduce its size to about 19MB, then is zipped and checked in with this commit.
Thanks for having a go at this. I haven't tried it yet but looks good. Seeing that the JGit repo is still 19MB after you took the time to clean it and to zip it, and checking where and how it is used in tests, I would rather not use it at all. I want to merge your changes as they fix many test failures, especially the hello-world repository. So this helps right now. In the long run I don't think the tests should use arbitrary repos from the internet but that we have synthesized repositories which have only the content needed for the various tests. So while we can go with the other two for the time being, as a starting point to replace them with proper ones at some point, if they are small enough, I feel a bit uneasy assing 19MB binary weight to the project for only a few tests. So I would rather leave that out and either rewrite the tests or disable them for now. |
Just my opinion, but as stated at #1275 (comment) , I recommend creating repositories at runtime. |
I think that there is merit in all three types of repositories for tests, prepared and checked in, created on the fly and external. For more complex tests or tests that require writing, putting together a repository during runtime makes sense, also in-memory. Again, maybe one repo can be used by multiple tests, so creating it might be part of a utility class instead of every single test. And for a handful of integration tests external repositories will be needed, I think. But that is for the future. |
My aim was to make the tests working again with minimum changes to them. Rewriting them to use repos created on the fly is a very big job that I don't have time for. Maybe @rnveach's idea would be suitable for new tests to be written, as we all know that more test cases are needed for the project, especially UI tests. |
I think zipping the JGit repository and adding 17MB is a bit much for the handful of tests that it is used for. I have therefore taken the JGit repository out of your commit. I have rewritten the commit, instead of adding a new one, so that the zip file doesn't become part of the history, taking up space. So your fixes are in, but the JGit repository is still pulled from the Internet. This makes most tests pass again. At a later time the tests using the JGit repository can be rewritten to use a different repo. I'm closing this pull request, as the changes are merged with commit e57a846. |
No description provided.