Unit test logs should be written to build path, not source path

laanwj commented at 9:07 am on October 23, 2019: member

When doing out-of-tree builds it’s not expected that the build (or tests) clutter the source path. All build output should be written to the build path (the current directory when configure is run) and subdirectories of that. (theoretically it should even be possible to have the source on a read-only mount, but that’s not really important imo, the point is to not clutter the git repository with build output)

However, after running make check there will be a while slew of *.log files in the source directory:

 0bitcoin$ ./autogen.sh
 1bitcoin$ make build
 2bitcoin$ cd build
 3bitcoin/build$ ../configure ...
 4bitcoin/build$ make check
 5bitcoin/build$ cd ..
 6bitcoin$ ls src/test/*.log
 7src/test/addrman_tests.cpp.log            src/test/compress_tests.cpp.log         src/test/netbase_tests.cpp.log          src/test/sigopcount_tests.cpp.log
 8src/test/allocator_tests.cpp.log          src/test/crypto_tests.cpp.log           src/test/net_tests.cpp.log              src/test/skiplist_tests.cpp.log
 9src/test/amount_tests.cpp.log             src/test/cuckoocache_tests.cpp.log      src/test/pmt_tests.cpp.log              src/test/streams_tests.cpp.log
10src/test/arith_uint256_tests.cpp.log      src/test/dbwrapper_tests.cpp.log        src/test/policyestimator_tests.cpp.log  src/test/sync_tests.cpp.log
11...

Either these files should be written to a temporary path (like the other test output), or to the build directory.

laanwj added the label Tests on Oct 23, 2019

laanwj added the label good first issue on Oct 23, 2019

sadrasabouri commented at 8:40 am on October 24, 2019: none

@laanwj Hi. I searched the code for this issue and i’m wondering if i should take this actions:

In Make.test.include changing all test/... to ../build/test/... which seems not to be an efficient solution :(.

OR

In Makefile:line 2241 if test -f "./$$f"; then dir=./; \ elif test -f "$$f"; then dir=; \ else dir="$(srcdir)/"; fi; \ tst=$$dir$$f; log='$@'; I think we should change $(srcdir) here to build directory so that test logs will save in build sub directory which seems much better.

laanwj commented at 11:35 am on October 24, 2019: member

In Make.test.include changing all test/… to ../build/test/… which seems not to be an efficient solution :(.

I think that would be incorrect. The source should stay in the source path.

I think we should change $(srcdir) here to build directory so that test logs will save in build sub directory which seems much better.

Yes, I think that would be better. The idea is to get the .log files to be written somewhere else, but keep everything else the same.

sadrasabouri commented at 6:28 am on October 31, 2019: none

Where can i change this? It seems this file (Makefile in src/) is created after make and does not exist before making process.

brakmic commented at 8:42 pm on November 23, 2019: contributor

Hi,

After some investigation and reading and trying out various Makefile settings, I think I’ve found the reason why the logs always land in tests directory.

It has to do with the fact that autogen.sh is automatically generating test-driver in build/aux.

test-driver itself has an option, –log-file, but because there is currently no way to access it before the compilation is kicked off, the settings fall back to their default: generate log files in current dir.

Here’s the snippet of test-driver’s params:

 0while test $# -gt 0; do
 1  case $1 in
 2  --help) print_usage; exit $?;;
 3  --version) echo "test-driver $scriptversion"; exit $?;;
 4  --test-name) test_name=$2; shift;;
 5  --log-file) log_file=$2; shift;;
 6  --trs-file) trs_file=$2; shift;;
 7  --color-tests) color_tests=$2; shift;;
 8  --expect-failure) expect_failure=$2; shift;;
 9  --enable-hard-errors) enable_hard_errors=$2; shift;;
10  --) shift; break;;
11  -*) usage_error "invalid option: '$1'";;
12   *) break;;
13  esac
14  shift
15done

Further down in its code there is also a line where the logs get written.

0# Test script is run here.
1"$@" >$log_file 2>&1

I’ve seen some projects keeping the test-driver under revision control, because they prefer to build & configure it without autogen.sh. Maybe this strategy could be applied here as well? Just in case it’s fitting to the overall architecture, of course.

Regards,

laanwj commented at 12:51 pm on November 24, 2019: member

@brakmic thanks for the thorough investigation!

glaksmono commented at 3:46 am on February 3, 2020: contributor

Anyone is working on this? Otherwise, I would like to take a stab on it

fanquake commented at 3:48 am on February 3, 2020: member

Anyone is working on this? Otherwise, I would like to take a stab on it @glaksmono Feel free to work on it.

glaksmono commented at 4:21 am on February 4, 2020: contributor

hey @fanquake, @laanwj,

Should we make a temp folder under ./tests/temp or ./src/tests/temp for all of the log files?

I presume that the temp folder should be created the first time we’re running make check?

Thoughts?

MarcoFalke commented at 1:47 pm on May 31, 2020: member

Just putting them in $build_patch/src/test/ should be enough, no?

MarcoFalke commented at 2:18 pm on June 26, 2020: member

Not sure how automake works, but there should be a symbol abs_top_builddir somewhere, which is the build path

yancyribbens commented at 2:20 pm on June 26, 2020: contributor

This issue has been around for a while so I thought I’d take a stab at it. @MarcoFalke thanks for the suggestion about using abs_top_builddir.

yancyribbens commented at 2:23 pm on June 26, 2020: contributor

@MarcoFalke Sorry for the confusion with the comments. I deleted my comment and in the 10 seconds it took me to write a new comment you replied xD

yancyribbens commented at 2:25 pm on June 26, 2020: contributor

@MarcoFalke could we move the discussion to the PR?

Psycho-Pirate commented at 8:25 pm on February 15, 2022: none

Is this issue fixed by @yancyribbens PR? I would like to work on this if something else needs to be done.

MarcoFalke referenced this in commit d2615312c1 on Mar 25, 2022

MarcoFalke commented at 12:55 pm on March 25, 2022: member

I think this can be closed now?

MarcoFalke removed the label good first issue on Mar 25, 2022

MarcoFalke closed this on Mar 31, 2022

sidhujag referenced this in commit d1042c8870 on Apr 2, 2022

sidhujag referenced this in commit 5e7cf64a5c on Apr 3, 2022

DrahtBot locked this on Mar 31, 2023

Unit test logs should be written to build path, not source path #17224