Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests failed in master branch #148

Open
versatildefuy opened this issue Oct 18, 2019 · 10 comments
Open

Tests failed in master branch #148

versatildefuy opened this issue Oct 18, 2019 · 10 comments

Comments

@versatildefuy
Copy link
Contributor

Problem:
Two tests fail executing npm test

  76 passing (8s)
  24 pending
  2 failing

  1) MongoDB Device registry
       "before each" hook:
     Uncaught Error: getaddrinfo ENOTFOUND http://unexistent.com
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:50:26)

  2) MongoDB Device registry
       "after each" hook for "should insert a new device in the "devices" collection":
     TypeError: Cannot read property 'db' of undefined
      at Context.<anonymous> (test/unit/server/device-registry-mongo-test.js:76:20)

The second test is included in the unit testing directory while it seems is testing a transaction with a mongo database.

I am interested in contributing some smalls modifications in order to make the server compatible with other OMA LwM2M libraries. In this way, in order to follow the contribution guidelines, I need to be able to run the tests without fail.

Thank you in advance

@fgalan
Copy link
Member

fgalan commented Oct 18, 2019

At which point of the code (i.e. githash commit) are you running the tests?

@versatildefuy
Copy link
Contributor Author

Master branch commit → 9145fda

@fgalan
Copy link
Member

fgalan commented Oct 21, 2019

That point in code seems to be a bit old (August 2nd). Could you checkout the last version of master and test again, please?

As a refrence, this is the result of the test in my local environment with the last version of master today (18d6ebc5933f8c098e58ca0a499c24aa2951eb5c):

  45 passing (43s)
  11 pending

@versatildefuy
Copy link
Contributor Author

How are you running the tests? Because following the Testing section in README.md I'm running even different number of tests.

@fgalan
Copy link
Member

fgalan commented Oct 24, 2019

Maybe I did a dirty and invalid test. Let's try again with the latest in master. Full script (skipping some longs parts):

$ git checkout master
...
$ git pull
...
$ git status   # to check I don't have any extra file in my working copy
On branch master
Your branch is up-to-date with 'origin/master'.
nothing to commit, working directory clean
$ npm run clean

> [email protected] clean /data/src/lwm2m-node-lib
> rm -rf package-lock.json && rm -rf node_modules && rm -rf coverage

$ npm install
...
$ npm test

  81 passing (16s)
  24 pending

(Sometime one of the test fails due to timeout, but I haven't pay too much importance to it, as I understand is a problem of my local enviornament as far as it works in travis. If I expand the timeout changing the following lines in packages.json it seems to work fine)

-    "test": "mocha --recursive 'test/**/*.js' --reporter spec --timeout 3000 --ui bdd --exit",
+    "test": "mocha --recursive 'test/**/*.js' --reporter spec --timeout 30000 --ui bdd --exit",

@fgalan
Copy link
Member

fgalan commented Oct 24, 2019

Do you have mongo installed and running in the host where you run the tests? In my case, I do, and, as far as I understand by .travis.yml, it's needed.

@versatildefuy
Copy link
Contributor Author

Ok, I ran MongoDB with docker and now all tests work. This requirement may be described in the testing documentation section.

On the other hand, all tests are under the common unit/ directory while most tests seem to be more integration, system or e2e tests.

@fgalan
Copy link
Member

fgalan commented Oct 28, 2019

Ok, I ran MongoDB with docker and now all tests work. This requirement may be described in the testing documentation section.

Documentation modified in PR #156. Please have a look and provide LGTM if you find it ok (or suggest and alternative wording).

On the other hand, all tests are under the common unit/ directory while most tests seem to be more integration, system or e2e tests.

Not sure of fully understand you... could you provide some more detail please?

@versatildefuy
Copy link
Contributor Author

IMHO, the fact that you need mongo for some tests shows that not all of your tests are unit. At least those check the integration with mongo, in a direct or indirect way.

In addition, other tests check the functionality by starting a client and a server and making assertions on any of the parts. I don't think this kind of tests can be classified as unit.

@fgalan
Copy link
Member

fgalan commented Oct 29, 2019

IMHO, the fact that you need mongo for some tests shows that not all of your tests are unit. At least those check the integration with mongo, in a direct or indirect way.

From a strict point of view, you are right. However, doing "pure" unit tests without mongo would need a mock for mongo, which make the test implementation more complex. Thus, using mongo in this case seems to be a good trade off between complexity and "unitiness".

In addition, other tests check the functionality by starting a client and a server and making assertions on any of the parts. I don't think this kind of tests can be classified as unit.

Which ones? Could provide a link to some, please? Anyway, probably is a case like the former: they aren't pure unit test but anyway provide valuable testability of some parts of the code (in fact, an integration test is usually more valuable than an unit test, as it is testing a case closer to real utilizanton of the software).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants