test: check if no NDCTL-related code is in use without PMem

[janekmi]

DAOS-18296

---

This change is 

[osalyk]

@osalyk reviewed 13 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @janekmi).

---

src/test/obj_ndctl_bb/TEST1 line 23 at r1 (raw file):

CREATE=c
OPEN=o

Why do you need this?

---

src/test/obj_ndctl_sds/TEST0 line 21 at r1 (raw file):

CREATE=c
OPEN=o

.

Code quote:

CREATE=c
OPEN=o

[janekmi]

@janekmi made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @osalyk).

---

src/test/obj_ndctl_bb/TEST1 line 23 at r1 (raw file):

Previously, osalyk (Oksana Sałyk) wrote…

Why do you need this?

For readability. TBH I don't like command lines which look like this exe file c vs exe file o. You have to read the exe source code to understand what just happened. At the same time, just for a test exe you do not want to employ a whole blown getopt. So, I created these monikers to leave some context on the Bash level without overcomplicating the test source code itself.

---

src/test/obj_ndctl_sds/TEST0 line 21 at r1 (raw file):

Previously, osalyk (Oksana Sałyk) wrote…

.

.

[osalyk]

@osalyk made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @janekmi).

---

src/test/obj_ndctl_bb/TEST1 line 23 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

For readability. TBH I don't like command lines which look like this exe file c vs exe file o. You have to read the exe source code to understand what just happened. At the same time, just for a test exe you do not want to employ a whole blown getopt. So, I created these monikers to leave some context on the Bash level without overcomplicating the test source code itself.

But from what I can see, you're not using the short form (are we going to use binaries outside of testing?).
In my opinion, there's no improvement in readability here, maybe I'm missing something.

[janekmi]

@janekmi made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @osalyk).

---

src/test/obj_ndctl_bb/TEST1 line 23 at r1 (raw file):

Previously, osalyk (Oksana Sałyk) wrote…

But from what I can see, you're not using the short form (are we going to use binaries outside of testing?).
In my opinion, there's no improvement in readability here, maybe I'm missing something.

The improvement is: c may mean anything you can think of:

• a verb starting with c e.g.
  • check
  • convert
  • compare
  • you name it
• a noun starting with c e.g.
  • connection
  • constructor
  • ...
• and, from experience with other PMDK test, anything else not starting with c e..g
  • open

So, by replacing something like this:

expect_abnormal_exit obj_ndctl_bb $FILE c
# with something like this
expect_abnormal_exit obj_ndctl_bb $FILE $CREATE

I hope to make clear that this line creates a pool.

Does it make sense?

[osalyk]

@osalyk made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @janekmi).

---

src/test/obj_ndctl_bb/TEST1 line 23 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

The improvement is: c may mean anything you can think of:

• a verb starting with c e.g.
  • check
  • convert
  • compare
  • you name it
• a noun starting with c e.g.
  • connection
  • constructor
  • ...
• and, from experience with other PMDK test, anything else not starting with c e..g
  • open

So, by replacing something like this:

expect_abnormal_exit obj_ndctl_bb $FILE c
# with something like this
expect_abnormal_exit obj_ndctl_bb $FILE $CREATE

I hope to make clear that this line creates a pool.

Does it make sense?

That's exactly how I understood you :)
But you use the second form in the tests (expect\_abnormal\_exit obj\_ndctl\_bb $FILE $CREATE), so I don't understand why you need an explanation when it's written explicitly.

[janekmi]

@janekmi made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @osalyk).

---

src/test/obj_ndctl_bb/TEST1 line 23 at r1 (raw file):

Previously, osalyk (Oksana Sałyk) wrote…

That's exactly how I understood you :)
But you use the second form in the tests (expect\_abnormal\_exit obj\_ndctl\_bb $FILE $CREATE), so I don't understand why you need an explanation when it's written explicitly.

Oh. So, you consider this bit:

CREATE=c
OPEN=o

a form of documentation? So you think it would be just fine to call expect_abnormal_exit obj_ndctl_bb $FILE c since it is written a few lines higher that c stands for create?

If it is the case. The benefit with using a variable instead of just writing a documentation is that there is no chance this tight coupling I proposed to get out of sync in case of a change. Whereas a documentation for a binary interface written in a different file can get easily overlooked.

[janekmi]

@janekmi made 1 comment.
Reviewable status: 9 of 13 files reviewed, 2 unresolved discussions (waiting on @grom72 and @osalyk).

---

src/test/obj_ndctl_bb/TEST1 line 23 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Oh. So, you consider this bit:

CREATE=c
OPEN=o

a form of documentation? So you think it would be just fine to call expect_abnormal_exit obj_ndctl_bb $FILE c since it is written a few lines higher that c stands for create?

If it is the case. The benefit with using a variable instead of just writing a documentation is that there is no chance this tight coupling I proposed to get out of sync in case of a change. Whereas a documentation for a binary interface written in a different file can get easily overlooked.

Added comments as agreed offline.

[osalyk]

@osalyk reviewed 4 files and all commit messages, made 2 comments, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @grom72).

---

src/test/obj_ndctl_bb/TEST1 line 23 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Added comments as agreed offline.

Thanks!