This is an attempt to create new tests for libsbuf as outlined in the JuniorJobs page. It creates new test cases for sbuf_get_flags, sbuf_set_flags, sbuf_clear_flags, and sbuf_new (positive case only).
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
Hi @ellisgen_gmail.com !
Could you please run clang-format on this file to resolve any formatting-related issues?
Comment Actions
Ran clang-format-devel against sbuf_core_test.c and generated the diff using git diff.
Comment Actions
Let me fix the formatting issues beforehand so the diff can be minimized to the functional changes...
Comment Actions
Please rebase / reapply the new code changes. I reformatted the sources with clang-format in rG991bd461625a2c521d5be4fd6938deed57f60972 so your proposed changes will be highlighted instead of the current diff--which is addressing some existing style bugs in addition to your functional changes.
Comment Actions
Mostly a style(9) nit: style(9) says C comments should be used (/* ... */)
| lib/libsbuf/tests/sbuf_core_test.c | ||
|---|---|---|
| 386–401 | (picking a line) should each item with Case X be its own separate testcase? | |
| 423–428 | This technically could be elided, resulting in less complicated conditionals: #if defined(HAVE_SBUF_SET_FLAGS) && defined(HAVE_SBUF_GET_FLAGS)
ATF_TP_ADD_TC(tp, sbuf_set_flags_test);
#if defined(HAVE_SBUF_CLEAR_FLAGS)
ATF_TP_ADD_TC(tp, sbuf_clear_flags_test);
ATF_TP_ADD_TC(tp, sbuf_flags_interaction_test);
#endif /* defined(HAVE_SBUF_CLEAR_FLAGS) */
#endif /* defined(HAVE_SBUF_SET_FLAGS) && defined(HAVE_SBUF_GET_FLAGS) */Not a blocking comment. Just an observation / potential low priority improvement. | |
| 433–436 | It's better to file issues which can be referenced more easily by others than add deadcode like this. | |
| lib/libsbuf/tests/sbuf_core_test.c | ||
|---|---|---|
| 433–436 |
Wait a second... it wasn't disabled before. Did you mean to leave it disabled? | |
| lib/libsbuf/tests/sbuf_core_test.c | ||
|---|---|---|
| 433–436 | It was disabled before, and I left it disabled. The reason is because I'm confused about how to properly implement the negative test. I implemented a test (with a couple of cases), but the test failed. The reason for the test failure was due to KASSERT not throwing errors, because I do not have a debug kernel (I think). And I'm also confused about not having runtime checks on the inputs to sbuf_new. Most of my experience is in higher level languages, and I know runtime checks on sbuf_new would impact performance (however slightly). So, it would appear that KASSERT in sbuf_new only throws when in debug. How do you write tests that pass in debug and non-debug? Or do you only run tests against debug? | |