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

Incorporate tests/test_* application into tests/unittests if it makes sense #1131

Closed
3 of 5 tasks
miri64 opened this issue May 9, 2014 · 16 comments
Closed
3 of 5 tasks
Labels
Area: tests Area: tests and testing framework Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented

Comments

@miri64
Copy link
Member

miri64 commented May 9, 2014

Identified candidates so far:

  • tests/test_bloom
  • tests/test_bloom_bytes
  • tests/test_netif
  • tests/test_posix_semaphore
  • tests/test_sha256
@OlegHahm
Copy link
Member

@phiros, we postpone this until the next release, right?

@phiros
Copy link
Member

phiros commented Dec 17, 2014

@OlegHahm : yes definitely. It is quite hard to transform the remaining tests into unit tests (at least with our current test setup).
The main reasons for this are:

  • test_bloom_bytes: this test is designed to test the performance of the bloom module. It should therefore become a performance test instead of a unit test.
  • tests/net_if: I think this could be transformed into a unit test BUT we need some kind of mocking mechanism in order to do so (we could also do it without but I think that would consume to much development time)
  • tests/posix_semaphore: I think we could test some properties of the posix semaphores module with proper mocking support, however we shouldn't only rely on tests when it comes to code which is inherently concurrent.

@LudwigKnuepfer LudwigKnuepfer modified the milestones: Release 2014.12, Release NEXT MAJOR Dec 17, 2014
@kushalsingh007
Copy link
Member

@authmillenon : So what are the part's that can be currently worked upon , Although the previous comment does explain the reason for shifting it to next release , however since it has been recently tagged as Newbie-Task-CandidateI think there might be some areas on which I could work upon , can you point out a few.

@kushalsingh007
Copy link
Member

It would be great if I could get some sort of direction to start the work.

@miri64
Copy link
Member Author

miri64 commented Mar 20, 2015

I think in the current version this applies only tests/libfixmath_unittests and tests/net_if. tests/libfixmath_unittests is notorious for having problems to download from the repository (which caused a lot of false positives on Travis e.g. yesterday and why it should remain seperated from the other unittests) and tests/net_if is on its way of being obsolete. But if you want to, you can go through the current sys modules and see if there are modules where no unittests are implemented at tests/unittests.

@miri64
Copy link
Member Author

miri64 commented Mar 20, 2015

(and where it makes sense. modules that involve a lot of threading or timers usually can't be tested this way for now for example)

@miri64
Copy link
Member Author

miri64 commented Mar 20, 2015

A quick evaluation later the following modules seem like suitable candidates:

  • hashes
  • hash_string
  • tm
  • utlist

@kushalsingh007
Copy link
Member

Thanks for having a look , working on the these now ;)

@kushalsingh007
Copy link
Member

I think along with the tests there needs to be some change in the code itself , for instance current hash string function
has no check to ensure that the value of variable hash does not exceed size of unsigned long,
so I am changing it by breaking the loop if size is greater than unsigned long size limit . So I am changing the code too.

@miri64
Copy link
Member Author

miri64 commented Mar 21, 2015

That is good, if errors like this surface :-), but please do the fixes in a seperate PR than the tests (or do the fixes + the tests for the module in module-wise).

@PeterKietzmann
Copy link
Member

@authmillenon I think the list is not up to date, right?

@miri64
Copy link
Member Author

miri64 commented Jul 6, 2015

Not really, no. But don't hesitate to update it ;-)

@PeterKietzmann
Copy link
Member

As far as I see it there are some unittests fot the new netif. There are still no unittests for the semaphores. I don't get why you listed bloom and bloom_bytes separately.

@miri64
Copy link
Member Author

miri64 commented Jul 7, 2015

Because those were seperate test applications in the tests/ directory at the time I opened the issue.

@OlegHahm OlegHahm removed this from the Release 2015.12 milestone Dec 2, 2015
@OlegHahm OlegHahm modified the milestones: Release 2015.12, Release 2016.03 Dec 2, 2015
@OlegHahm OlegHahm modified the milestones: Release 2016.07, Release 2016.04 Mar 28, 2016
@kYc0o
Copy link
Contributor

kYc0o commented Jul 26, 2016

Somehow this was reaaaally forgotten... moving for the next release.

@kYc0o kYc0o modified the milestones: Release 2016.10, Release 2016.07 Jul 26, 2016
@miri64
Copy link
Member Author

miri64 commented Oct 31, 2016

To be honest, reevaluating this issue I would say it is fixed. Except bloom_bytes and posix_semaphore they are all ported to unittests, but the two tests use threads and timers respectively, so they are less suitable for unittests anyway.

@miri64 miri64 closed this as completed Oct 31, 2016
@miri64 miri64 added the Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented label Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented
Projects
None yet
Development

No branches or pull requests

8 participants