8000 External repos zipped by chirontt · Pull Request #1309 · gitblit-org/gitblit · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Closed
wants to merge 3 commits into from

Conversation

chirontt
Copy link

No description provided.

chirontt added 3 commits April 27, 2019 14:51
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.
@flaix
Copy link
Member
flaix commented May 20, 2019

Thanks for having a go at this. I haven't tried it yet but looks good.
How large are the ambition and gitective repositories?

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.

@rnveach
Copy link
rnveach commented May 20, 2019

Just my opinion, but as stated at #1275 (comment) , I recommend creating repositories at runtime.
Example: rnveach@d4b7fec#diff-40dc38b8e1cc8224c3b9602657444c93R82

@flaix
Copy link
Member
flaix commented May 21, 2019

I think that there is merit in all three types of repositories for tests, prepared and checked in, created on the fly and external.
I do not believe in creating a repository on the fly for each and every test. I think that is too much overhead for many tests. For a lot of unit tests it is good to have one or two curated repositories put on disk that they can run against, especially if they only read from them. One curated repository can server a lot of tests.

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.

@chirontt
Copy link
Author

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.

@flaix
Copy link
Member
flaix commented Jun 7, 2019

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.

@flaix flaix closed this Jun 7, 2019
@flaix flaix added this to the 1.9.0 milestone Nov 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0