Page MenuHomePhabricator

Add unit tests for seeder's parse_name()
AbandonedPublic

Authored by nakihito on Mon, Oct 21, 19:45.

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

Adds some unit tests for the seeder's parse_name() function.

Depends on D4275

Test Plan

In your make build dir:

../configure
make check
../configure --with-seeder=false
make check

seeder_tests.cpp should not show up during testing

ls src/seeder/

This should only show the test/ directory.

ls src/seeder/test

This should show nothing.

In your cmake build dir:

cmake -GNinja ..
ninja check

rm -r ./*

You can skip this step, but there will be leftover seeder directory and files in the next part if you do.

cmake -GNinja -DBUILD_BITCOIN_SEEDER=OFF ..
ninja check
ls src

There should be no seeder directory. If you skipped the rm -r ./* step then you can verify with:

ls -al src
src/test/test_bitcoin --run_test=parse_name_simple

The seeder directory should have an earlier time stamp than everything else.
The command should fail with the following error message:

Test setup error: no test cases matching filter or all test cases were disabled

Diff Detail

Repository
rABC Bitcoin ABC
Branch
ParseNameUnitTest
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7857
Build 13734: Bitcoin ABC Buildbot
Build 13733: arc lint + arc unit

Event Timeline

nakihito created this revision.Mon, Oct 21, 19:45
Owners added a reviewer: Restricted Owners Package.Mon, Oct 21, 19:45
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Oct 21, 19:45
nakihito planned changes to this revision.Mon, Oct 21, 19:45
nakihito requested review of this revision.Mon, Oct 21, 19:45
nakihito edited the summary of this revision. (Show Details)
nakihito retitled this revision from Add unit tests for parse_name() to Add unit tests for seeder's parse_name().Mon, Oct 21, 19:48
jasonbcox requested changes to this revision.Mon, Oct 21, 20:44
jasonbcox added inline comments.
src/Makefile.test.include
149 ↗(On Diff #13632)

This and the test plan are missing BUILD_BITCOIN_SEEDER

src/seeder/test/seeder_tests.cpp
34 ↗(On Diff #13632)

Why does this need a static_cast? Won't uint8_t(0) suffice?

This revision now requires changes to proceed.Mon, Oct 21, 20:44
nakihito planned changes to this revision.Mon, Oct 21, 23:09
jasonbcox added inline comments.Wed, Oct 23, 00:44
src/seeder/test/seeder_tests.cpp
34 ↗(On Diff #13632)

Ignore this. I missed that this was a typedef.

nakihito updated this revision to Diff 13678.Wed, Oct 23, 23:31

Rebased and add if guards for when the project is built without the seeder. Also added CMake.

nakihito planned changes to this revision.Wed, Oct 23, 23:31
nakihito edited the test plan for this revision. (Show Details)Wed, Oct 23, 23:55
nakihito requested review of this revision.Thu, Oct 24, 19:52

It would be beneficial to build another executable for the seeder tests.

nakihito planned changes to this revision.Fri, Oct 25, 01:27

It would be beneficial to build another executable for the seeder tests.

I'll try and do that.

nakihito requested review of this revision.Fri, Nov 8, 19:34

Executable added in D4413.

nakihito planned changes to this revision.Fri, Nov 8, 23:22
nakihito abandoned this revision.Sat, Nov 9, 01:01

See D4418.