UI-DEN-5638-1 UI-DEN-5638_Squish Migration

closed on 15 Feb 21

Author

Reviewers

UI-DEN-5638_Squish Migration

Activity

Details
Objectives
General Comments 7
Unresolved
Resolved
Number of files included: 41

testsuites 0
- Folder shared/scripts 0
  - File names.py 0 Remove
- Folder simulator 0
  - File run.py 0 Remove
- Folder tst_AlarmCleared 0
  - File test.py 0 Remove
- Folder tst_AlarmStatusData 0
  - File test.py 0 Remove
- Folder tst_AlarmTriggered 0
  - File test.py 0 Remove
- Folder tst_Alarm_Colors 0
  - File test.py 0 Remove
- Folder tst_CANBusFaultCount 0
  - File test.py 0 Remove
- Folder tst_ConfirmPrimingBegin 0
  - File test.py 0 Remove
- Folder tst_CreateTreatment 0
  - File test.py 0 Remove
- Folder tst_DGDrainPumpData 0
  - File test.py 0 Remove
- Folder tst_DGHeatersData 0
  - File test.py 0 Remove
- Folder tst_DGLoadCellReadingsData 0
  - File test.py 0 Remove
- Folder tst_DGOperationMode 0
  - File test.py 0 Remove
- Folder tst_DGPressureData 0
  - File test.py 0 Remove
- Folder tst_DGROPumpData 0
  - File test.py 0 Remove
- Folder tst_DGReservoirData 0
  - File test.py 0 Remove
- Folder tst_DGTemperaturesData 0
  - File test.py 0 Remove
- Folder tst_DGValvesStatesData 0
  - File test.py 0 Remove
- Folder tst_DebugText 0
  - File test.py 0 Remove
- Folder tst_HDBloodFlowData 0
  - File test.py 9 Remove
- Folder tst_HDInletFlowData 0
  - File test.py 0 Remove
- Folder tst_HDOperationModeData 0
  - File test.py 0 Remove
- Folder tst_HDOutletFlowData 0
  - File test.py 0 Remove
- Folder tst_HDPressureOcclusionData 0
  - File test.py 0 Remove
- Folder tst_HomeScreen 0
  - File test.py 0 Remove
- Folder tst_Internals 0
  - File test.py 0 Remove
- Folder tst_ServiceShutdown 0
  - File test.py 5 Remove
- Folder tst_TreatmentSalineData 0
  - File test.py 0 Remove
- Folder tst_TreatmentScreen 0
  - File test.py 0 Remove
- Folder tst_TreatmentStatesData 0
  - File test.py 6 Remove
- Folder tst_Treatment_Adjustment_BloodDialysate 0
  - File test.py 0 Remove
- Folder tst_Treatment_Adjustment_Duration 0
  - File test.py 0 Remove
- Folder tst_Treatment_Adjustment_Saline 0
  - File test.py 0 Remove
- Folder tst_Treatment_Adjustment_Ultrafiltration 0
  - File test.py 0 Remove
- Folder tst_Treatment_BloodDialysateFlowRate 0
  - File test.py 0 Remove
- Folder tst_Treatment_ParametersRange 0
  - File test.py 0 Remove
- Folder tst_Treatment_PressureOcclusion 0
  - File test.py 0 Remove
- Folder tst_Treatment_Section_BloodDialysate 0
  - File test.py 0 Remove
- Folder tst_Treatment_Time 0
  - File test.py 0 Remove
- Folder tst_Treatment_Ultrafiltration 0
  - File test.py 0 Remove
- File requirements.txt 3 Remove

#permalink

Details

Warning: no files are visible, they have all been filtered.

Participant	Role	Time Spent	Comments	Latest Comment
plucia (deleted user)	Author	5h 6m	14	Nothing has changed about the usage of the SquishQt API, ...
Behrouz NematiPour	Reviewer - Complete	2h 33m	16	Still don't feel comfortable with this solution and haven...
pmontazemi (deleted user)	Reviewer - Complete	9m
Total		7h 49m	30

Linked issues
- DEN-5638

#permalink

Objectives

There are no specific objectives for this review.

Branches in review

#permalink

Issues Raised From Comments

Key	Summary	State	Assignee

#permalink

General Comments

plucia (deleted user)

Related dialin code review: http://dvm-linux02:8060/cru/DIALIN-DEN-5638-1

24 Nov 20

plucia (deleted user)

As of 12/30/2020, with these changes: Code Coverage: 100% Squish Tests Passin...

As of 12/30/2020, with these changes:
Code Coverage: 100%
Squish Tests Passing: 100%

30 Dec 20

Behrouz NematiPour

It is wrong to create an instance of HDSimulator each time you want to use an...

It is wrong to create an instance of HDSimulator each time you want to use any function in that class.
1 - this class has not been defined as a static class, therefore, all the listers in class will be listening for each instance and the question is which one is going to respond, or all will respond, and then we will have multiple responses.
2 -This class not only should have the "cmd_" functions as static
3 - But also it needs to be a singleton class if want to have listers.

The design and usage of the class are not correct.

24 Jan 21

Behrouz NematiPour

Resolved

27 Jan 21

plucia (deleted user)

Limiting the HDSimulator to static functions prevents you from maintaining an...

Limiting the HDSimulator to static functions prevents you from maintaining any state between successive calls and is incompatible with the DenaliCanMessenger. It is an improvement to hold state information and allows us to use the DenaliCanMessenger. Using the DenaliCanMessenger and dialin eliminates the need to maintain duplicated codebases and allows for bidirectional messaging. Your squish folder was doing the same thing as dialin but on a smaller scale.

1 - The test scripts are run in a consecutive manner, not in parallel. Each test should be self contained.
2 - (See statement above). This comment is about dialin, not testsuites.
3 - (See statement above). This comment is about dialin, not testsuites.

25 Jan 21

Behrouz NematiPour

I don't mean to limit the HDSimulator to static functions, it can have static...

I don't mean to limit the HDSimulator to static functions, it can have static and normal methods.

1 - The test scripts are run in a consecutive manner, not in parallel. Each test should be self contained.

I agree in this case but the general implementation and design of HDSimulator lead to that misuse of the class.

26 Jan 21

plucia (deleted user)

We are in agreement that each test should be self contained. To ensure each t...

We are in agreement that each test should be self contained.
To ensure each test is self-contained, each test needs to have a separate instance of HDSimulator.
The design of dialin is not the focus of this review...

26 Jan 21

Behrouz NematiPour

RESOLVED

27 Jan 21

Click to load diff

Notation	Description
[Crucible Review CR-1\|CR-1]	Creates a link to a Crucible review or FishEye artifact using the internal key reference for the item. Example: [Crucible Review CR-1\|CR-1]
[Atlassian Crucible\|http://atlassian.com/crucible]	Creates a link to an external resource, special characters that come after the URL and are not part of it must be separated with a space. External links are denoted with an arrow icon. Example: Atlassian Crucible
{cs:id=1\|rep=repository-key}	Render a link to the changeset Example: 1
bold	Makes text appear bold.
_italic_	Makes text appear in italics.
+underline+	Makes text appear underlined.
??citation??	Makes text appear in citation form.
-strikethrough-	Makes text appear ~~struck through~~.
^superscript^	Makes text appear in ^superscript.
~subscript~	Makes text appear in _subscript.
{{ monospaced }}	Placing double curly-brackets around text makes it appear `monospaced`.
{code} // Some comments here public String getFoo() { return foo; } {code}	The code macro displays a preformatted block for showing code with syntax highlighting. Example: // Some comments here public String getFoo() { return foo; }
bq. Block Quote	To make an entire paragraph into a block quotation, place "bq. " before it. Example: Block Quote
{quote} here is quoteable content to be quoted {quote}	Quote a block of text that's longer than one paragraph. Example: here is quoteable content to be quoted
{color:red} look ma, red text!{color}	Changes the color of a block of text. Example: look ma, red text!
* A list with bullets * Second item ** Indented item	Example: A list with bullets Second item Indented item
# A list with numbers # Second item ## Indented item	Example: A list with numbers Second item Indented item

UI-DEN-5638-1

UI-DEN-5638_Squish Migration

UI-DEN-5638-1 30

Details

Linked issues

Objectives

Branches in review

Issues Raised From Comments

General Comments

plucia (deleted user)

plucia (deleted user)

Behrouz NematiPour

plucia (deleted user)

Behrouz NematiPour

plucia (deleted user)

Behrouz NematiPour

Log time

UI-DEN-5638_Squish Migration

UI-DEN-5638-1 30

Summarize the review outcomes (optional)

Details

Linked issues

Objectives

Branches in review

Issues Raised From Comments

General Comments

plucia (deleted user)

plucia (deleted user)

Behrouz NematiPour

plucia (deleted user)

Behrouz NematiPour

plucia (deleted user)

Behrouz NematiPour

Log time