From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: "Daniel Sangorrin" References: <1537858732-32952-1-git-send-email-daniel.sangorrin@toshiba.co.jp> <1537858732-32952-2-git-send-email-daniel.sangorrin@toshiba.co.jp> In-Reply-To: Date: Fri, 28 Sep 2018 10:43:32 +0900 Message-ID: <005801d456cc$aa44a6d0$fecdf470$@toshiba.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: ja Subject: Re: [Fuego] [PATCH 01/11] rebuild: add if_source_changed flag List-Id: Mailing list for the Fuego test framework List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tim.Bird@sony.com, fuego@lists.linuxfoundation.org Hi Tim, > Does the ftc run-test code depend on this, or can I take them out of = order? > I'd like to apply that patch, but consider this one a bit longer. No, it doesn't depend on this. > I'd like to keep these issues separate, if possible, because while = this > series includes things that are very valuable, I think there are = issues > with a confusion between the software under test and test software, > for the Functional.kernel_build test. > > I think I supported the idea to move the actual build steps from > Functional.kernel_build's test_run() function to the test_build() > function. But now I am not sure that was the correct decision. I understand what you say, but the if_source_changed flag is useful for = any test, not just for kernel_build. In addition, even if we move the "build test" back to test_run, the = source code will still be fetched during the build phase. The user can choose between cloning the entire kernel repository each = time (Rebuild=3DTrue) or only pulling the latest changes = (if_source_changed). Where we run the build test (make uImage etc) , = either on test_build or on test_run, is a separate issue from how we are = fetching the source code, which is what this patch is mainly about. Yest another separate issue is whether we want to build from pristine = state (do make mrproper) or reuse the cache of objects as much as = possible. > Logically: the kernel build test should consist of: > build the kernel, get compiler report of errors as the testlog, parse = those errors > there should not be a deploy step at all, as it is purely > a host-side operation. >=20 > Well - the deploy should consist of storing the build artifacts = somewhere that > can be used for other tests that utilize this software. The results (the kernel and modules) are copied to a local directory. = The user can choose to copy them to the log directory, or to = /var/lib/tftpboot (after installing tftpd on Fuego's docker) for = example. The next step would be to create a Functional.kernel_deploy test to = deploy the kernel to the target directly (using put), or indirectly = through a server (ftp, html, tftp, LAVA..). Finally, we could create Functional.reboot to reboot the DUT and check = if the boot was succesful. This "check" can be many things: check it = booted in less than 10 seconds, check we get the expected prompt, check = we can connect to the board, check Functional.hello_world works, etc.. > I guess that's what the current deploy step does, but it's not > quite as explicit about it as I'd like. Could you elaborate a bit on that? > A separate boot test might consist of: > build the kernel, deploy the kernel, boot the DUT, get console logs = from DUT as the > testlog, parse for any errors > (also, detect if the DUT did not boot at all) That could be done like this: ftc run-test -b myboard -t Funtional.kernel_build -s myboard-tftpboot [Opt] ftc run-test -b myboard -t Functional.kernel_deploy ftc run-test -b myboard -t Functional.reboot -s myboard-console-prompt The first part would build the kernel and copy it into = /var/lib/tftpboot. Optionally, we can leave the deploy step to = Functional.kernel_deploy. Then, Functional.reboot would power cycle the board (or ssh reboot it) = and check that it boots succesfully. =20 > In this case the kernel is the test program (it executes on the board = to perform the > test, and > produces the log used to check for results). >=20 > The part to build the kernel might be: 'ftc test-run -b $BOARD -t > Functional.kernel_build'. > I can imagine other tests (eg. a static kernel size checker) that = would want a built > kernel, but > would do different operations on it, rather than executing it. That could be easily done: ftc test-run -b $BOARD -t Functional.kernel_build -s logdir ftc test-run -b $BOARD -t Functional.kernel_sizecheck -s = latestbuildnumber > The bottom line is this: I know I recommended it, but I think this = solves a > different problem for the kernel_build test than it solves for other = tests. > And this difference is making me want to take a step back before I = apply this > change. >=20 > I want to consider this more, before creating a new rebuild flag. I understand what you say, but the rebuild flag is more about what to do = with the source code. Do we want to clone the kernel again or just do a git pull? The default is if_source_changed, but the user can set --rebuid true If we keep a hardcoded "export Rebuild=3Dtrue" then the user cannot = decide. Thanks, Daniel > -- Tim >=20 > > -----Original Message----- > > From: Daniel Sangorrin > > > > if_source_changed allows rebuilding only when the test source > > has changed which is the main use case. For that reason, > > we set it as the default option. > > > > FIXTHIS: consider conflicts when updating via git pull > > > > Signed-off-by: Daniel Sangorrin > > --- > > engine/scripts/ftc | 10 +++--- > > engine/scripts/functions.sh | 40 > +++++++++++++++++++--- > > engine/tests/Functional.kernel_build/fuego_test.sh | 12 ------- > > 3 files changed, 42 insertions(+), 20 deletions(-) > > > > diff --git a/engine/scripts/ftc b/engine/scripts/ftc > > index 7de2b70..b675dea 100755 > > --- a/engine/scripts/ftc > > +++ b/engine/scripts/ftc > > @@ -96,13 +96,14 @@ where_help =3D \ > > command_help =3D { > > "add-jobs": ("Adds jobs to Jenkins.", > > """Usage: ftc add-jobs -b [,board2...] [-p | = -t > > -s ] > > - [-k ] [--rebuild ] [--reboot = ] > > + [-k ] [--rebuild = ] > [--reboot > > ] > > [--precleanup ] [--postcleanup ] > > Example: ftc add-jobs -b docker -p testplan_docker > > Example: ftc add-jobs -b docker -t Benchmark.Dhrystone -k 5m = --rebuild > > false > > board,testplan,testpec: use list-board/plans/specs to see the = available > > ones. > > timeout: integer with a suffix from 'smhd' (seconds, minutes, = hours, days). > > rebuild: if true rebuild the test source even if it was already = built. > > + if_source_changed can be used to rebuild the test only when its = source > > changes. > > reboot: if true reboot the board before the test. > > precleanup: if true clean the board's test folder before the = test. > > postcleanup: if true clean the board's test folder after the = test. > > @@ -241,7 +242,7 @@ You can obtain a list of run_ids for runs on the = local > > system with > > > > "run-test": ("Run a test on a board.", > > """Usage: ftc run-test -b -t [-s ] [-p = ] > > - [-k ] [--rebuild ] [--reboot = ] > > + [-k ] [--rebuild ] = [--reboot > > ] > > [--precleanup ] [--postcleanup ] > > [--dynamic-vars python_dict] > > Run the indicated test on the specified board. Use the > > @@ -250,6 +251,7 @@ indicated spec, if one is provided. > > board,spec: use list-board/specs to see the available ones. > > timeout: integer with a suffix from 'smhd' (seconds, minutes, = hours, days). > > rebuild: if true rebuild the test source even if it was already = built. > > +if_source_changed can be used to rebuild the test only when its = source > > changes. > > reboot: if true reboot the board before the test. > > precleanup: if true clean the board's test folder before the test. > > postcleanup: if true clean the board's test folder after the test. > > @@ -652,7 +654,7 @@ class test_class: > > 'timeout' : '30m', > > 'spec' : 'default', > > 'reboot' : 'false', > > - 'rebuild' : 'false', > > + 'rebuild' : 'if_source_changed', > > 'precleanup' : 'true', > > 'postcleanup' : 'true' > > } > > @@ -1489,7 +1491,7 @@ def do_add_jobs(conf, options): > > rebuild =3D options[options.index('--rebuild') + 1] > > except IndexError: > > error_out('Rebuild option not provided after = --rebuild') > > - if rebuild not in ['true', 'false']: > > + if rebuild not in ['true', 'false', 'if_source_changed']: > > error_out("Invalid rebuild option '%s'" % rebuild) > > options.remove(rebuild) > > options.remove('--rebuild') > > diff --git a/engine/scripts/functions.sh = b/engine/scripts/functions.sh > > index 952e9a4..05bdbc1 100755 > > --- a/engine/scripts/functions.sh > > +++ b/engine/scripts/functions.sh > > @@ -94,6 +94,9 @@ function untar { > > *) echo "Unknown $tarball file format. Not unpacking."; = return 1;; > > esac > > tar ${key}xf $TEST_HOME/$tarball --strip-components=3D1 > > + > > + # record md5sum for possible source code updates > > + md5sum $TEST_HOME/$tarball > fuego_tarball_src_md5sum > > } > > > > # Unpacks/clones the test source code into the current directory. > > @@ -321,6 +324,11 @@ function build_error { > > abort_job "Build failed: $@" > > } > > > > +function prepare_build_dir { > > + find . -maxdepth 1 -mindepth 1 ! -name "fuego.build.lock" = -print0 | xargs > > -0 rm -rf > > + unpack || abort_job "Error while unpacking" > > +} > > + > > # process Rebuild flag, and unpack test sources if necessary. > > function pre_build { > > cd ${WORKSPACE} > > @@ -341,11 +349,35 @@ function pre_build { > > > > lock_build_dir > > > > - if [ "$Rebuild" =3D "false" ] && [ -e = fuego_test_successfully_built ]; then > > - echo "The test is already built" > > + if [ "$Rebuild" =3D "true" ]; then > > + prepare_build_dir > > + elif [ "$Rebuild" =3D "false" ]; then > > + if [ -e fuego_test_successfully_built ]; then > > + echo "The test was already successfully built" > > + else > > + prepare_build_dir > > + fi > > + elif [ "$Rebuild" =3D "if_source_changed" ]; then > > + if [ -e fuego_tarball_src_md5sum ]; then > > + if md5sum -c fuego_tarball_src_md5sum; then > > + echo "No updates to the source code tarball" > > + else > > + echo "The source code tarball got updated, = rebuilding" > > + prepare_build_dir > > + fi > > + elif [ -d ".git" ]; then > > + echo "Trying to update the source code with git pull" > > + if git pull | grep "Already up-to-date"; then > > + echo "No source code updates" > > + else > > + echo "The source code was updated, rebuilding" > > + rm -f fuego_test_successfully_built > > + fi > > + else > > + prepare_build_dir > > + fi > > else > > - find . -maxdepth 1 -mindepth 1 ! -name "fuego.build.lock" = -print0 | > > xargs -0 rm -rf > > - unpack || abort_job "Error while unpacking" > > + abort_job "Rebuild flag $Rebuild is not recognized." > > fi > > } > > > > diff --git a/engine/tests/Functional.kernel_build/fuego_test.sh > > b/engine/tests/Functional.kernel_build/fuego_test.sh > > index 0b4ac3c..2d36055 100755 > > --- a/engine/tests/Functional.kernel_build/fuego_test.sh > > +++ b/engine/tests/Functional.kernel_build/fuego_test.sh > > @@ -18,21 +18,9 @@ function test_pre_check { > > if [[ "$FUNCTIONAL_KERNEL_BUILD_TARGET" =3D=3D *"uImage"* ]]; = then > > is_on_sdk mkimage > > fi > > - > > - # Force the kernel to be rebuilt > > - # That's the whole point of the test - don't let Fuego skip the = build. > > - export Rebuild=3DTrue > > } > > > > function test_build { > > - # make sure we have the latest source code. > > - cd "$WORKSPACE/$JOB_BUILD_DIR" > > - if [ -d ".git" ]; then > > - git pull || echo "WARNING: git pull failed" > > - else > > - echo "WARNING: no git repository, assuming you used a = tarball" > > - fi > > - > > # Config > > if [ -z "$FUNCTIONAL_KERNEL_BUILD_CONFIG" ]; then > > FUNCTIONAL_KERNEL_BUILD_CONFIG=3D"defconfig" > > -- > > 2.7.4 > > > > _______________________________________________ > > Fuego mailing list > > Fuego@lists.linuxfoundation.org > > https://urldefense.proofpoint.com/v2/url?u=3Dhttps- > > = 3A__lists.linuxfoundation.org_mailman_listinfo_fuego&d=3DDwICAg&c=3DfP4tf= - > > -1dS0biCFlB0saz0I0kjO5v7- > > GLPtvShAo4cc&r=3DjjTc71ylyJg68rRxrFQuDFMMybIqPCnrHF85A- > > GzCRg&m=3D8ywuWmFMqK6gIy82Qwk2GhR4SAydFRy_Hm1rE1T6C0o&s=3DzVM > > w_UdedEmHCijaZOExVLun0-MxGge7Ey5eo0gkIeY&e=3D