signetlabdei / lorawan

An ns-3 module for simulation of LoRaWAN networks
GNU General Public License v2.0
190 stars 132 forks source link

Tests, tests, tests! #49

Closed DvdMgr closed 2 years ago

DvdMgr commented 5 years ago

Tests are a vital component of the module, and a great way to get started if you want to understand what's going on under the hood.

As of now, we have tests for the PHY layer model implementation, but more is needed for the MAC layer and for Network Server functionality.

If you want to contribute towards this, you are welcome! Please feel free to use this issue to ask questions on how to get started - I will be happy to assist you.

gaurivn commented 3 years ago

@DvdMgr, I would like to work on this issue. Could you please advise me on the same

DvdMgr commented 3 years ago

Sure! A good starting point would be to make sure tests run correctly, both on the master and on the develop branches. If this is not the case, we need to identify the test that does fails and understand why. You can follow the readme instructions to compile the module and run the tests!

gaurivn commented 3 years ago

Thank you Sir for replying. I will do accordingly.

gaurivn commented 3 years ago

Sure! A good starting point would be to make sure tests run correctly, both on the master and on the develop branches. If this is not the case, we need to identify the test that does fails and understand why. You can follow the readme instructions to compile the module and run the tests!

The lorawan test suite passed on the develop branch with no errors, but on master branch, it gave errors for lorawan/model/building-penetration-loss.cc, please find the screenshot attached below: lorawan_test-develop

DvdMgr commented 3 years ago

Ok, as I expected, it's failing due to a change in the ns-3 APIs - the IsOutdoor method was deprecated, !IsIndoor() should be used instead. Can you fix it? I'll be happy to merge your pull request!

gaurivn commented 3 years ago

Ok, as I expected, it's failing due to a change in the ns-3 APIs - the IsOutdoor method was deprecated, !IsIndoor() should be used instead. Can you fix it? I'll be happy to merge your pull request!

I used !IsIndoor() instead but I am still getting more errors, I will try fix those..please find the screenshots attached below: Screenshot from 2021-03-17 16-38-56 Screenshot from 2021-03-17 16-39-02

DvdMgr commented 3 years ago

It looks like the problem is that in the tests we are trying to compare an instance of ns3::Time with an integer. This is happening inside the NS_TEST_EXPECT_MSG_EQ macro - it should be enough to change that 0 to a Seconds(0), so that it becomes a Time instance!

gaurivn commented 3 years ago

Thank you, I made the change and the one error remaining was due to deprecated method - MakeMobilityModelConsistent() in complete-network-example.cc Screenshot from 2021-03-18 18-24-20 I commented it out and committed again and the lorawan test suite passed without errors. Please let me know if I need to make more changes.