luftfartsverket / reqstool-client

Reqstool is a tool for managing requirements with related software verification cases (aka tests) and verification results (test results)
https://luftfartsverket.github.io/reqstool-client/
MIT License
2 stars 0 forks source link

feat: Changes for requirements without implementation #78

Closed lfvbarcus closed 4 months ago

lfvbarcus commented 5 months ago

This PR adds functionality to write requirements that do not require a source code implementation.

The current state is as follows

The requirement.yml field now supports an optional key (name pending) called implemented_in_src. If this key is omitted, the default value is set to True

This alters how we count statistics, as some requirements could be considered complete even without source code implementation.

The final report could now look like this:

image

In this example there are four requirements, three of them does not expect a source code implementation (REQ_101, REQ_1337 & REQ_1339) The only requirement that expects one is REQ_201 and is included for the purpose of highlighting the changes made.

There are a lot of small changes under the hood here. Let's break them down so that we can make sure that my thinking is correct.

1) The most notable change is that REQ_101 and REQ_1337 is marked as complete despite the fact that the Implementation column is marked as N/A, since their tests are completed. This is also accounted for in the "implemented and verified" column, as two out of four requirements are completed. 2) REQ_1339 is not marked as completed since it has failing tests. It is however not currently included (because of it's state as incomplete) in the statistics in the table with the headers "implemented and verified", "implemented" and "not implemented" I'm not sure if we should count it as "Not implemented" as it per definition should not have one. Please feel free to give me some feedback on this. 3) The REQ_201 is implemented, and as it is the only requirement that should have one, The "implemented" column is displaying: 1 (100%) indicating that all requirements that should be implemented is in fact implemented.
4) If a requirement that is marked to not have an implementation actually has one, an error is thrown during parsing with the following message:

image

5) We should also agree on the name of the key in the requirements.yml file. It is currently named implemented_in_src but I think that I would prefer expect_implementation.

Since we are introducing requirements with a quite different expected behavior, the columns "implemented and verified", "implemented" and "not implemented" is somewhat misleading. I'm not sure on what we should call them, so I have tried my best to accommodate the changes to the current format of the status tables.

@lfvjimisola @lfvdavid @rlinhem What do you think about these changes?

lfvdavid commented 5 months ago

Under total requirements maybe we could add another column after "not implemented" called "should not be implemented" or something.

This way we have statistics for all different "states" a requirement can have.

Edit: This does not account for verified / unverified requirements that does not expect an implementation.

Call the column something that reflects that a requirement is verified and does not expect an implementation, then leave any requirements not verified and without an implementation under "not implemented".

If we want to track it in the same way as implemented / implemented and verified we need to add two more columns reflecting how many requirements does not expect an implementation and how many of those has been verified with tests.

lfvbarcus commented 5 months ago

@lfvjimisola It seems as your comment from yesterday is missing? Did you forget to submit it?

I have forgotten the proposed structure of the table for the statistics.

Would something like this make sense?

image

lfvjimisola commented 5 months ago

We did that in Excel if I recall right.

lfvbarcus commented 4 months ago

We did that in Excel if I recall right.

  • "Code", "N/A"
  • Total, Verified

Is this what we are looking for?

image

if it's not, then it better be very close, as editing those headers are about to make a grown man cry.

lfvjimisola commented 4 months ago

We did that in Excel if I recall right.

  • "Code", "N/A"
  • Total, Verified

Is this what we are looking for?

image

if it's not, then it better be very close, as editing those headers are about to make a grown man cry.

We said that it should say "Implementations" above right? Not sure if "Combined" is the correct one.

Implementations Code (Total / Verified), N/A (Total / Verified), Not implemented

lfvbarcus commented 4 months ago

We did that in Excel if I recall right.

  • "Code", "N/A"
  • Total, Verified

Is this what we are looking for? image if it's not, then it better be very close, as editing those headers are about to make a grown man cry.

We said that it should say "Implementations" above right? Not sure if "Combined" is the correct one.

Implementations Code (Total / Verified), N/A (Total / Verified), Not implemented

This is the best I can do

image

lfvjimisola commented 4 months ago

This is the best I can do

Is there somewhere where it would be easy to indicate that it's 'ALL GREEN'?

lfvbarcus commented 4 months ago

This is the best I can do

Is there somewhere where it would be easy to indicate that it's 'ALL GREEN'?

Perhaps if we where to color code the CODE and N/A headlines red or green like we do in the table? CODE is green = all code related requirements are done. CODE is red = incomplete code requirements

Or should the Implementations header be color coded instead? Green for all pass, otherwise red?

lfvjimisola commented 4 months ago

This is the best I can do

Is there somewhere where it would be easy to indicate that it's 'ALL GREEN'?

Perhaps if we where to color code the CODE and N/A headlines red or green like we do in the table? CODE is green = all code related requirements are done. CODE is red = incomplete code requirements

Or should the Implementations header be color coded instead? Green for all pass, otherwise red?

We could do both right? The Implementations is the aggregate of "CODE && N/A".

lfvbarcus commented 4 months ago

This is the best I can do

Is there somewhere where it would be easy to indicate that it's 'ALL GREEN'?

Perhaps if we where to color code the CODE and N/A headlines red or green like we do in the table? CODE is green = all code related requirements are done. CODE is red = incomplete code requirements Or should the Implementations header be color coded instead? Green for all pass, otherwise red?

We could do both right? The Implementations is the aggregate of "CODE && N/A".

Yes, let's do both. I'll sort it out after lunch

lfvbarcus commented 4 months ago

This is how it could look now.

All Requirements are implemented and have passing tests:

image

Only requirements that are implemented in code and have passing tests:

image

Only requirements that should not have an implementation has passing tests:

image

Neither has all passing tests or are implemented correctly:

image

Is this clear enough or should I edit this further? @lfvjimisola

lfvjimisola commented 4 months ago

I like it! It more intuitive I think. What do you think?

lfvdavid commented 4 months ago

May be nitpicking but I would expand the code header and include not implemented under it.

|                         implementations                          |
|-------------------------------------------------------------------|
|                     code               |           N/A            |
|----------------------------------------|--------------------------|
| total   | verified    | not implemented| total   | verified       |
lfvbarcus commented 4 months ago

May be nitpicking but I would expand the code header and include not implemented under it. t

|                         implementations                          |
|-------------------------------------------------------------------|
|                     code               |           N/A            |
|----------------------------------------|--------------------------|
| total   | verified    | not implemented| total   | verified       |

That depends on what we should count as "not implemented". I would like to think that it should count the number of requirements (both code and n/a) that are not yet completed. However, I realize that's not what it does right now, I think that it only counts the number of code elements that's left to implement. It's an easy fix if we want to count them combined though. Perhaps with another naming it we should decide on that route.

lfvbarcus commented 4 months ago

I like it! It more intuitive I think. What do you think?

I think it looks much better! Great suggestion!

rlinhem commented 4 months ago

May be nitpicking but I would expand the code header and include not implemented under it. t

|                         implementations                          |
|-------------------------------------------------------------------|
|                     code               |           N/A            |
|----------------------------------------|--------------------------|
| total   | verified    | not implemented| total   | verified       |

That depends on what we should count as "not implemented". I would like to think that it should count the number of requirements (both code and n/a) that are not yet completed. However, I realize that's not what it does right now, I think that it only counts the number of code elements that's left to implement. It's an easy fix if we want to count them combined though. Perhaps with another naming it we should decide on that route.

I would argue that it's more interesting to know what's 'verified' and 'not verified' in the total. Then we can consider both 'code' and 'N/A' requirements together. 'implemented' and 'not implemented' is only relevant for 'code' requirements.

lfvbarcus commented 4 months ago

May be nitpicking but I would expand the code header and include not implemented under it. t

|                         implementations                          |
|-------------------------------------------------------------------|
|                     code               |           N/A            |
|----------------------------------------|--------------------------|
| total   | verified    | not implemented| total   | verified       |

That depends on what we should count as "not implemented". I would like to think that it should count the number of requirements (both code and n/a) that are not yet completed. However, I realize that's not what it does right now, I think that it only counts the number of code elements that's left to implement. It's an easy fix if we want to count them combined though. Perhaps with another naming it we should decide on that route.

I would argue that it's more interesting to know what's 'verified' and 'not verified' in the total. Then we can consider both 'code' and 'N/A' requirements together. 'implemented' and 'not implemented' is only relevant for 'code' requirements.

There's another take on this. Could you provide a mockup of what you would like to change? I'm not really sure what to change based on what you have written.

lfvbarcus commented 4 months ago

After some input from Robert, this is what he requested (mock-up in asciidoc):

image

The code section should display

  1. Total number of requirements that should be implemented in code
  2. The number of code related requirements that is currently implemented
  3. The number of code related requirements that is currently verified
  4. The number of code related requirements that is not currently verified

The N/A section should display:

  1. Total number of requirements that should not be implemented in code
  2. The number of non-code related requirements that is currently verified
  3. The number of non-code related requirements that is not currently verified

The CODE, N/A and Implementations will be highlighted with green and red color just like before.

I would like some input on this suggestion before I edit those tables again, to preserve my sanity :)

@lfvjimisola @lfvdavid @rlinhem

lfvdavid commented 4 months ago

I think this is better, but do we need the not verified section under N/A?

We have total requirements, and verified requirements, not verified is implied by the diff between the two

lfvjimisola commented 4 months ago
  1. I don't mind the "Not verified" column. It's perhaps the most important information - "What do we have left?".

The CODE, N/A and Implementations will be highlighted with green and red color just like before.

  1. I don't see "Implementations" is that a spanning row above CODE and N/A?

  2. The example is not correct - it does not add upp. Just so that we are in agreement. For Code there are 2 verified but only 1 implement. I can't see that that can happen. I believe that this is true:

lfvbarcus commented 4 months ago
  1. I don't mind the "Not verified" column. It's perhaps the most important information - "What do we have left?".

The CODE, N/A and Implementations will be highlighted with green and red color just like before.

  1. I don't see "Implementations" is that a spanning row above CODE and N/A?
  2. The example is not correct - it does not add upp. Just so that we are in agreement. For Code there are 2 verified but only 1 implement. I can't see that that can happen. I believe that this is true:
  • Total >= Implemented
  • Total >= Verified
  • Total >= Not Verified
  • Total >= Verified + Not Verified
  • Implemented >= Verified
  • Implemented >= Not Verified
  • Implemented >= Verified + Not Verified
  1. Alright, then I can add that column.
  2. Correct, the overall look has not changed. The headers will look like I have demonstrated before.
  3. You are right, the example does not reflect how it should count, I just entered some numbers to illustrate the looks. For a requirement to be classed as verified both passing test(s) and at least one implementation is needed.
lfvbarcus commented 4 months ago

I think this is better, but do we need the not verified section under N/A?

We have total requirements, and verified requirements, not verified is implied by the diff between the two

The addition was a suggestion from @rlinhem as he would argue that the not verified is more interesting than the number that is verified.

lfvbarcus commented 4 months ago

This is where I'm currently at:

image

lfvjimisola commented 4 months ago

This is where I'm currently at:

  1. I think we discussed this before but replace "STATUS" with spanning row cell for "REQUIREMENTS: <# of requirements".
  2. Span implementations over whole row and use upper case
  3. Change CODE and N/A to Code and N/A
  4. I think that "Implemented" should be between Total and Verified as it is more logical (see https://github.com/Luftfartsverket/reqstool-client/pull/78#issuecomment-2138780773).
lfvbarcus commented 4 months ago

This is where I'm currently at:

  1. I think we discussed this before but replace "STATUS" with spanning row cell for "REQUIREMENTS: <# of requirements".
  2. Span implementations over whole row and use upper case
  3. Change CODE and N/A to Code and N/A
  4. I think that "Implemented" should be between Total and Verified as it is more logical (see feat: Changes for requirements without implementation #78 (comment)).

Alright.

image

It this looks good, then I need to do some cleanup before I can merge this.

lfvjimisola commented 4 months ago

It this looks good, then I need to do some cleanup before I can merge this.

Cosmetic:

  1. Requirement is still in lower-case.
  2. Req Id -> ID (now that we have a header)
  3. Automated Test -> Manual Tests
  4. Manual Test -> Manual Tests

I have a question (sorry!): would it not be useful to have the total for Automated Tests and Manuals Tests below each column? The cell would contain "Px Sy Fz Mn". It would give an overall of how our testing status is. There is enough room left-to-right to with in data since the Requirements table is narrower than the Implementations.

lfvbarcus commented 4 months ago

It this looks good, then I need to do some cleanup before I can merge this.

Cosmetic:

  1. Requirement is still in lower-case.
  2. Req Id -> ID (now that we have a header)
  3. Automated Test -> Manual Tests
  4. Manual Test -> Manual Tests

I have a question (sorry!): would it not be useful to have the total for Automated Tests and Manuals Tests below each column? The cell would contain "Px Sy Fz Mn". It would give an overall of how our testing status is. There is enough room left-to-right to with in data since the Requirements table is narrower than the Implementations.

I can fix the cosmetic stuff, but changing anything regarding the tests has to be a ticket of its own as it is out of scope for what I've been trying to implement here.

@rlinhem @lfvdavid @lfvJonas what do you guys think about Jimisolas suggestion?

lfvbarcus commented 4 months ago

Alright, the cosmetics are now done!

image