Project

General

Profile

Actions

Bug #3418

closed

Unit tests requiring root should be skipped when run as normal user

Added by susmit shannigrahi almost 9 years ago. Updated about 8 years ago.

Status:
Closed
Priority:
Normal
Category:
Faces
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:

Description

NFD has a few unit tests that needs to be run as root. When run as normal user, these unit tests should be skipped and not throw errors.

../daemon/face/udp-factory.cpp(203): fatal error in "std::shared_ptr<nfd::face::Face> nfd::UdpFactory::createMulticastFace(const Endpoint&, const Endpoint&, const string&)": std::runtime_error: Cannot bind multicast face to virbr0: Operation not permitted
../tests/daemon/mgmt/face-manager-process-config.t.cpp(176): error in "ProcessSectionUdp": exception thrown by parseConfig(CONFIG, false)
../tests/daemon/mgmt/face-manager-process-config.t.cpp(305): error in "ProcessSectionUdpMulticastReinit": exception thrown by parseConfig(CONFIG_WITH_MCAST, false)
../tests/daemon/mgmt/general-config-section.t.cpp(79): error in "UserAndGroupConfig": exception thrown by configFile.parse(CONFIG, true, "test-general-config-section")
../tests/daemon/mgmt/general-config-section.t.cpp(93): error in "NoUserConfig": exception thrown by configFile.parse(CONFIG, true, "test-general-config-section")
Actions #1

Updated by Junxiao Shi over 8 years ago

  • Category set to Management
  • Target version set to v0.5

An alternate to the suggested solution is to require sudo in README-dev.md "running unit tests" section.

Actions #2

Updated by Junxiao Shi over 8 years ago

  • Category changed from Management to Faces
Actions #3

Updated by susmit shannigrahi over 8 years ago

Junxiao Shi wrote:
An alternate to the suggested solution is to require sudo in README-dev.md "running unit tests" section.

Not really.

If I want to run unit tests when packaging on a Fedora(or any other) build-system, I won't have sudo. Running unit tests for packaging is required for sanity checking.

Actions #4

Updated by Davide Pesavento over 8 years ago

susmit shannigrahi wrote:

Junxiao Shi wrote:
An alternate to the suggested solution is to require sudo in README-dev.md "running unit tests" section.

Not really.

If I want to run unit tests when packaging on a Fedora(or any other) build-system, I won't have sudo.

+1. Requiring sudo to run unit tests is not a valid solution for packagers.

Actions #5

Updated by Junxiao Shi over 8 years ago

  • Assignee set to Weiwei Liu

@Weiwei: consult @Davide on how to properly solve this bug.

Actions #6

Updated by Weiwei Liu over 8 years ago

  • Status changed from New to In Progress
Actions #7

Updated by Weiwei Liu over 8 years ago

Could someone give me some idea about how to solve this bug? Am I supposed to modify the waf script?

Thanks a lot.

Actions #8

Updated by Alex Afanasyev over 8 years ago

No. I think unit tests should be aware of whether they are running as root or normal users. Anything that requires root should be skipped with a message.

There is an example in tests/core/privilege-helper.t.cpp.

You also need to update jenkins script in .jenkins.d/20-tests.sh

Actions #9

Updated by Weiwei Liu over 8 years ago

@Alex, thanks for your reply.

Do you mean for each test case that needs to run as root, I need to add the following code to skip it:
if (::geteuid() != 0) {
BOOST_TEST_MESSAGE("This test case needs to be run as super user, skipping");
return;
}

If this is the case why do we still need to update .jenkins.d/20-tests.sh?

Besides, how am I supposed to reproduce this bug?

Thank you very much.

Actions #10

Updated by Weiwei Liu over 8 years ago

I tried to run the NFD daemon tests with ./build/unit-tests-daemon -l test_suite, but I didn't get the errors as showed in this bug, instead, for each failed test case, I'm getting the following error:

../tests/daemon/mgmt/fib-manager.t.cpp:158: Entering test case "UnknownFaceId"
1415684132.000000 INFO: [StrategyChoice] setDefaultStrategy /localhost/nfd/strategy/best-route/%FD%04
../src/security/sec-tpm-file.cpp:395: fatal error: in "virtual ndn::Block ndn::SecTpmFile::signInTpm(const uint8_t *, size_t, const ndn::Name &, ndn::DigestAlgorithm)": boost::exception_detail::clone_implboost::exception_detail::error_info_injector >: FileStore: error opening file for reading: build/tmp-files/test-home/.ndn/ndnsec-tpm-file/zb5krffLzGeCok2Q5zuqJb0P8b5Fc2%cGMybeezt7WQ=.pri
../tests/daemon/mgmt/fib-manager.t.cpp:158: last checkpoint: "UnknownFaceId" entry.
../tests/daemon/mgmt/fib-manager.t.cpp:158: Leaving test case "UnknownFaceId"; testing time: 5192us

Does anyone know what's going wrong?

Updated by susmit shannigrahi over 8 years ago

Weiwei Liu wrote:
I tried to run the NFD daemon tests with ./build/unit-tests-daemon -l test_suite, but I didn't get the errors as showed in this bug, instead, for each failed test case, I'm getting the following error:

[...]

Does anyone know what's going wrong?

Per our discussion yesterday, this happens in only Fedora. I haven't personally tested any other distribution.

Actions #12

Updated by Weiwei Liu over 8 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 0 to 80
Actions #13

Updated by Junxiao Shi over 8 years ago

Should we add a step to Jenkins build script that executes unit testing without root and without adding capabilities?

Actions #14

Updated by Junxiao Shi over 8 years ago

  • Status changed from Code review to Resolved
  • % Done changed from 80 to 100

Patch is merged. This issue is waiting for an answer to note-13 question, and waiting for Susmit's test on his original environment.

Actions #15

Updated by Junxiao Shi over 8 years ago

  • Status changed from Resolved to In Progress
  • Assignee changed from Weiwei Liu to Davide Pesavento
  • % Done changed from 100 to 50

20160607 conference call decides to run all tests without added capabilities, and then re-run those tests that need sudo with added capabilities.

Actions #16

Updated by Davide Pesavento about 8 years ago

  • Status changed from In Progress to Code review
  • % Done changed from 50 to 90
Actions #17

Updated by Junxiao Shi about 8 years ago

  • Description updated (diff)
  • Status changed from Code review to Closed
  • % Done changed from 90 to 100
Actions

Also available in: Atom PDF