* [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis @ 2020-05-13 13:23 Đoàn Trần Công Danh 2020-05-13 13:23 ` [PATCH 1/2] t1509: correct i18n test Đoàn Trần Công Danh ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-13 13:23 UTC (permalink / raw) To: git; +Cc: Đoàn Trần Công Danh It's our long desire to enable t1509 in CI system [1] This is here to enable it. [1]: https://lore.kernel.org/git/1FEDCA48-FE77-44C3-8C4A-65B4C435E6B3@gmail.com/ Đoàn Trần Công Danh (2): t1509: correct i18n test CI: enable t1509 in Travis and GitHub Actions ci/lib.sh | 13 +++++++++++++ ci/run-docker-build.sh | 3 +++ t/t1509-root-work-tree.sh | 4 ++-- 3 files changed, 18 insertions(+), 2 deletions(-) -- 2.26.2.672.g232c24e857 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] t1509: correct i18n test 2020-05-13 13:23 [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis Đoàn Trần Công Danh @ 2020-05-13 13:23 ` Đoàn Trần Công Danh 2020-05-13 16:43 ` Junio C Hamano 2020-05-13 13:23 ` [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions Đoàn Trần Công Danh 2020-05-13 16:41 ` [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis Junio C Hamano 2 siblings, 1 reply; 8+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-13 13:23 UTC (permalink / raw) To: git; +Cc: Đoàn Trần Công Danh git-init(1)'s messages is subjected to i18n. They should be tested by test_i18n* family. Fix them. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- t/t1509-root-work-tree.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t1509-root-work-tree.sh b/t/t1509-root-work-tree.sh index 553a3f601b..fd2f7abf1c 100755 --- a/t/t1509-root-work-tree.sh +++ b/t/t1509-root-work-tree.sh @@ -221,7 +221,7 @@ test_expect_success 'setup' ' rm -rf /.git && echo "Initialized empty Git repository in /.git/" > expected && git init > result && - test_cmp expected result + test_i18ncmp expected result ' test_vars 'auto gitdir, root' ".git" "/" "" @@ -246,7 +246,7 @@ test_expect_success 'setup' ' cd / && echo "Initialized empty Git repository in /" > expected && git init --bare > result && - test_cmp expected result + test_i18ncmp expected result ' test_vars 'auto gitdir, root' "." "" "" -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] t1509: correct i18n test 2020-05-13 13:23 ` [PATCH 1/2] t1509: correct i18n test Đoàn Trần Công Danh @ 2020-05-13 16:43 ` Junio C Hamano 0 siblings, 0 replies; 8+ messages in thread From: Junio C Hamano @ 2020-05-13 16:43 UTC (permalink / raw) To: Đoàn Trần Công Danh; +Cc: git Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > git-init(1)'s messages is subjected to i18n. > They should be tested by test_i18n* family. > > Fix them. > > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > t/t1509-root-work-tree.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t1509-root-work-tree.sh b/t/t1509-root-work-tree.sh > index 553a3f601b..fd2f7abf1c 100755 > --- a/t/t1509-root-work-tree.sh > +++ b/t/t1509-root-work-tree.sh > @@ -221,7 +221,7 @@ test_expect_success 'setup' ' > rm -rf /.git && > echo "Initialized empty Git repository in /.git/" > expected && > git init > result && > - test_cmp expected result > + test_i18ncmp expected result > ' > > test_vars 'auto gitdir, root' ".git" "/" "" > @@ -246,7 +246,7 @@ test_expect_success 'setup' ' > cd / && > echo "Initialized empty Git repository in /" > expected && > git init --bare > result && > - test_cmp expected result > + test_i18ncmp expected result > ' > > test_vars 'auto gitdir, root' "." "" "" OK. It shows how rarely (if ever) this test script is actually exercised that we need fixes like these. Well done. Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions 2020-05-13 13:23 [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis Đoàn Trần Công Danh 2020-05-13 13:23 ` [PATCH 1/2] t1509: correct i18n test Đoàn Trần Công Danh @ 2020-05-13 13:23 ` Đoàn Trần Công Danh 2020-05-13 16:51 ` Junio C Hamano 2020-05-13 16:41 ` [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis Junio C Hamano 2 siblings, 1 reply; 8+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-13 13:23 UTC (permalink / raw) To: git; +Cc: Đoàn Trần Công Danh Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- ci/lib.sh | 13 +++++++++++++ ci/run-docker-build.sh | 3 +++ 2 files changed, 16 insertions(+) diff --git a/ci/lib.sh b/ci/lib.sh index dac36886e3..e9c22ae718 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -113,6 +113,7 @@ then export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save" export GIT_TEST_OPTS="--verbose-log -x --immediate" MAKEFLAGS="$MAKEFLAGS --jobs=2" + t1509_allowed=YES elif test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI" then CI_TYPE=azure-pipelines @@ -162,6 +163,7 @@ then echo "::add-matcher::ci/git-problem-matcher.json" test linux-musl = "$jobname" || MAKEFLAGS="$MAKEFLAGS TEST_SHELL_PATH=/bin/sh" + t1509_allowed=YES else echo "Could not identify CI type" >&2 env >&2 @@ -184,6 +186,17 @@ export DEVELOPER=1 export DEFAULT_TEST_TARGET=prove export GIT_TEST_CLONE_2GB=true +if test "$t1509_allowed" = YES +then + case "$jobname" in + osx-*) ;; + *) + chmod a+w / || sudo chmod a+w / || true + export IKNOWWHATIAMDOING=YES + ;; + esac +fi + case "$jobname" in linux-clang|linux-gcc) if [ "$jobname" = linux-gcc ] diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh index 8d47a5fda3..026afe275a 100755 --- a/ci/run-docker-build.sh +++ b/ci/run-docker-build.sh @@ -58,6 +58,8 @@ else test -n "$cache_dir" && chown -R $HOST_UID:$HOST_UID "$cache_dir" fi +chmod a+w / + # Build and test command $switch_cmd su -m -l $CI_USER -c " set -ex @@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c " export GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB' export MAKEFLAGS='$MAKEFLAGS' export cache_dir='$cache_dir' + export IKNOWWHATIAMDOING=YES cd /usr/src/git test -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove make -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions 2020-05-13 13:23 ` [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions Đoàn Trần Công Danh @ 2020-05-13 16:51 ` Junio C Hamano 2020-05-13 23:52 ` Đoàn Trần Công Danh 0 siblings, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2020-05-13 16:51 UTC (permalink / raw) To: Đoàn Trần Công Danh; +Cc: git Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> > --- > ci/lib.sh | 13 +++++++++++++ > ci/run-docker-build.sh | 3 +++ > 2 files changed, 16 insertions(+) > > diff --git a/ci/lib.sh b/ci/lib.sh > index dac36886e3..e9c22ae718 100755 > --- a/ci/lib.sh > +++ b/ci/lib.sh > @@ -113,6 +113,7 @@ then > export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save" > export GIT_TEST_OPTS="--verbose-log -x --immediate" > MAKEFLAGS="$MAKEFLAGS --jobs=2" > + t1509_allowed=YES > elif test -n "$SYSTEM_COLLECTIONURI" || test -n "$SYSTEM_TASKDEFINITIONSURI" > then > CI_TYPE=azure-pipelines > @@ -162,6 +163,7 @@ then > echo "::add-matcher::ci/git-problem-matcher.json" > test linux-musl = "$jobname" || > MAKEFLAGS="$MAKEFLAGS TEST_SHELL_PATH=/bin/sh" > + t1509_allowed=YES > else > echo "Could not identify CI type" >&2 > env >&2 > @@ -184,6 +186,17 @@ export DEVELOPER=1 > export DEFAULT_TEST_TARGET=prove > export GIT_TEST_CLONE_2GB=true > > +if test "$t1509_allowed" = YES > +then > + case "$jobname" in > + osx-*) ;; > + *) > + chmod a+w / || sudo chmod a+w / || true > + export IKNOWWHATIAMDOING=YES Eeeww ;-) This makes readers wonder where we did not enable the test and why. Perhaps throw in a matching t1509_allowed=NO in the azure thing for completeness? Also, do we want to give a more descriptive name than t1509 to the variable, say, ROOT_WORK_TREE_TEST_ALLOWED? > diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh > index 8d47a5fda3..026afe275a 100755 > --- a/ci/run-docker-build.sh > +++ b/ci/run-docker-build.sh > @@ -58,6 +58,8 @@ else > test -n "$cache_dir" && chown -R $HOST_UID:$HOST_UID "$cache_dir" > fi > > +chmod a+w / > + > # Build and test > command $switch_cmd su -m -l $CI_USER -c " > set -ex > @@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c " > export GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB' > export MAKEFLAGS='$MAKEFLAGS' > export cache_dir='$cache_dir' > + export IKNOWWHATIAMDOING=YES > cd /usr/src/git > test -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove > make Big EWWWWWWwwww. Do we need to do this for _all_ tests, not selectively only while running t1509? This makes me worried as a test by mistake can easily corrupt the VM and invalidating the tests; I know we get a fresh one every time, so there is no permanent harm done by corrupting it, but having one fewer thing we have to worry about is always better than having one more thing. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions 2020-05-13 16:51 ` Junio C Hamano @ 2020-05-13 23:52 ` Đoàn Trần Công Danh 2020-05-13 23:59 ` Junio C Hamano 0 siblings, 1 reply; 8+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-13 23:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On 2020-05-13 09:51:56-0700, Junio C Hamano <gitster@pobox.com> wrote: > Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > > +if test "$t1509_allowed" = YES > > +then > > + case "$jobname" in > > + osx-*) ;; > > + *) > > + chmod a+w / || sudo chmod a+w / || true > > + export IKNOWWHATIAMDOING=YES > > Eeeww ;-) This makes readers wonder where we did not enable the > test and why. Perhaps throw in a matching > > t1509_allowed=NO > > in the azure thing for completeness? I was thinking about allowing people set it via environment variable and check, but it seems too risky, now. Perhaps, always reset it to NO before the checking for $CI_TYPE, and enable it selectively for only Travis, and GitHub Actions. I didn't enable it for Azure because I can't assure it ;). > Also, do we want to give a more descriptive name than t1509 to the > variable, say, ROOT_WORK_TREE_TEST_ALLOWED? Yeah, I think all caps is better for this risky variable. I think using T1509_ROOT_WORK_TREE_TEST_ALLOWED is better, to point out which test is risky. But it require future tests with root work-tree must be written in t1509, since it's rare usecase, It'd be fine, I think. > > > diff --git a/ci/run-docker-build.sh b/ci/run-docker-build.sh > > index 8d47a5fda3..026afe275a 100755 > > --- a/ci/run-docker-build.sh > > +++ b/ci/run-docker-build.sh > > @@ -58,6 +58,8 @@ else > > test -n "$cache_dir" && chown -R $HOST_UID:$HOST_UID "$cache_dir" > > fi > > > > +chmod a+w / > > + > > # Build and test > > command $switch_cmd su -m -l $CI_USER -c " > > set -ex > > @@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c " > > export GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB' > > export MAKEFLAGS='$MAKEFLAGS' > > export cache_dir='$cache_dir' > > + export IKNOWWHATIAMDOING=YES > > cd /usr/src/git > > test -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove > > make > > Big EWWWWWWwwww. Do we need to do this for _all_ tests, not > selectively only while running t1509? This makes me worried as a > test by mistake can easily corrupt the VM and invalidating the > tests; I know we get a fresh one every time, so there is no > permanent harm done by corrupting it, but having one fewer thing we > have to worry about is always better than having one more thing. Perhaps pass this variable all the way down from ci/lib.sh? Adding another variable into t1509 (except T1509_*) doesn't make it less risky. Or should we add T1509_ prefix to this env var? -- Danh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions 2020-05-13 23:52 ` Đoàn Trần Công Danh @ 2020-05-13 23:59 ` Junio C Hamano 0 siblings, 0 replies; 8+ messages in thread From: Junio C Hamano @ 2020-05-13 23:59 UTC (permalink / raw) To: Đoàn Trần Công Danh; +Cc: git Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: >> > >> > +chmod a+w / >> > + >> > # Build and test >> > command $switch_cmd su -m -l $CI_USER -c " >> > set -ex >> > @@ -68,6 +70,7 @@ command $switch_cmd su -m -l $CI_USER -c " >> > export GIT_TEST_CLONE_2GB='$GIT_TEST_CLONE_2GB' >> > export MAKEFLAGS='$MAKEFLAGS' >> > export cache_dir='$cache_dir' >> > + export IKNOWWHATIAMDOING=YES >> > cd /usr/src/git >> > test -n '$cache_dir' && ln -s '$cache_dir/.prove' t/.prove >> > make >> >> Big EWWWWWWwwww. Do we need to do this for _all_ tests, not >> selectively only while running t1509? This makes me worried as a >> test by mistake can easily corrupt the VM and invalidating the >> tests; I know we get a fresh one every time, so there is no >> permanent harm done by corrupting it, but having one fewer thing we >> have to worry about is always better than having one more thing. > > Perhaps pass this variable all the way down from ci/lib.sh? > Adding another variable into t1509 (except T1509_*) doesn't make it > less risky. > Or should we add T1509_ prefix to this env var? I was not worried about any environment variable, but the "let's make the root directory writable by anybody during _all_ tests", when we need such a crazy permission bits on the filesystem only while running t1509 and not any other time, stood out as extremely yucky. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis 2020-05-13 13:23 [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis Đoàn Trần Công Danh 2020-05-13 13:23 ` [PATCH 1/2] t1509: correct i18n test Đoàn Trần Công Danh 2020-05-13 13:23 ` [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions Đoàn Trần Công Danh @ 2020-05-13 16:41 ` Junio C Hamano 2 siblings, 0 replies; 8+ messages in thread From: Junio C Hamano @ 2020-05-13 16:41 UTC (permalink / raw) To: Đoàn Trần Công Danh; +Cc: git Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > It's our long desire to enable t1509 in CI system [1] > > This is here to enable it. Yuck. It's not like having a single repository at /.git that govern everything on the machine is a usecase we care about, but if it is easily ported with just two patches, and the task has already been done, then why not? ;-) > > [1]: https://lore.kernel.org/git/1FEDCA48-FE77-44C3-8C4A-65B4C435E6B3@gmail.com/ > > Đoàn Trần Công Danh (2): > t1509: correct i18n test > CI: enable t1509 in Travis and GitHub Actions > > ci/lib.sh | 13 +++++++++++++ > ci/run-docker-build.sh | 3 +++ > t/t1509-root-work-tree.sh | 4 ++-- > 3 files changed, 18 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-05-13 23:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-13 13:23 [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis Đoàn Trần Công Danh 2020-05-13 13:23 ` [PATCH 1/2] t1509: correct i18n test Đoàn Trần Công Danh 2020-05-13 16:43 ` Junio C Hamano 2020-05-13 13:23 ` [PATCH 2/2] CI: enable t1509 in Travis and GitHub Actions Đoàn Trần Công Danh 2020-05-13 16:51 ` Junio C Hamano 2020-05-13 23:52 ` Đoàn Trần Công Danh 2020-05-13 23:59 ` Junio C Hamano 2020-05-13 16:41 ` [PATCH 0/2] CI: Enable t1509 on GitHub Actions and Travis Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).