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

Migrated 98 Unittest Tests to FHEM compatible prove test #940

Merged
merged 38 commits into from
Feb 13, 2021

Conversation

sidey79
Copy link
Contributor

@sidey79 sidey79 commented Feb 8, 2021

  • Please check if the PR fulfills these requirements
  • Tests for the changes have been added / modified (needed for for bug fixes / features)
  • commandref has been added / updated (needed for bug fixes / features)
  • CHANGED has been updated (needed for bug fixes / features)
  • What kind of change does this PR introduce?

It changes the way tests are running to a more Perl natural way, using prove and a process for every testfile.

Testfiles of the libs are migrated because they shoud not be stored under t/FHEM because the are outside the FHEM namespace.

  • What is the current behavior? (You can also link to an open issue here)

(1)
Tests are running using 98_Unittest Module and a shell script which tries to catch the results
All was bundeld into a makefile.

(2)
Some subs are referencing to FHEMWeb instance. If No instance ist provided, the sub crashes.

(3)
Result value from prepare_flash ist not returned

  • What is the new behavior (if this is a feature change)?

(1)
All tests are running via prove.

(2)
Before calling subs from FHEMWeb the existance ist checked.

(3)
Result value is given back as return

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

  • Other information:

@codecov
Copy link

codecov bot commented Feb 8, 2021

Codecov Report

Merging #940 (78549ee) into master (82f7eea) will increase coverage by 0.99%.
The diff coverage is 91.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #940      +/-   ##
==========================================
+ Coverage   56.18%   57.17%   +0.99%     
==========================================
  Files          66      103      +37     
  Lines        8075     8358     +283     
  Branches     1295     1308      +13     
==========================================
+ Hits         4537     4779     +242     
- Misses       2695     2714      +19     
- Partials      843      865      +22     
Flag Coverage Δ
fhem 47.18% <91.38%> (-44.19%) ⬇️
modules 57.17% <91.38%> (-34.20%) ⬇️
perl 91.42% <0.00%> (+0.04%) ⬆️
unittest ?
unittests 57.17% <91.38%> (-34.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
FHEM/lib/SD_Protocols.pm 78.72% <0.00%> (-0.74%) ⬇️
t/FHEM/00_SIGNALduino/01_SIGNALduino_Parse_MC.t 100.00% <ø> (ø)
t/FHEM/00_SIGNALduino/01_SIGNALduino_Parse_MN.t 100.00% <ø> (ø)
t/FHEM/00_SIGNALduino/01_SIGNALduino_Parse_MS.t 100.00% <ø> (ø)
t/FHEM/00_SIGNALduino/01_SIGNALduino_Parse_MU.t 100.00% <ø> (ø)
t/FHEM/00_SIGNALduino/01_SIGNALduino_Set_sendMsg.t 100.00% <ø> (ø)
t/FHEM/00_SIGNALduino/test_loadprotohash-ok.pm 80.00% <ø> (ø)
t/SD_ProtocolData/01_verifyFSK.t 98.77% <ø> (ø)
t/SD_Protocols/00_use.t 100.00% <ø> (ø)
t/SD_Protocols/01_LengthInRange.t 100.00% <ø> (ø)
... and 120 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82f7eea...2201b19. Read the comment docs.

@sidey79 sidey79 changed the title Migrated 98 Unittest Tests to FHEM compatible Probe test Migrated 98 Unittest Tests to FHEM compatible prove test Feb 8, 2021
@sidey79 sidey79 force-pushed the unittest-2-prove branch 2 times, most recently from 04a2a15 to d8d88de Compare February 8, 2021 20:46
@sidey79
Copy link
Contributor Author

sidey79 commented Feb 8, 2021

t/FHEM/10_SD_GT/09_parseDatat.t                                (Wstat: 256 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t/FHEM/14_SD_UT/09_parseDatat.t                                (Wstat: 256 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t/FHEM/14_SD_WS/09_parseDatat.t                                (Wstat: 256 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t/FHEM/14_SD_WS09/09_parseDatat.t                              (Wstat: 256 Tests: 2 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1

Da sind ein paar Tests, da stimmen die Werte aus dem JSON nicht.
Ich hab mir das nicht näher angesehen, teilweise sieht es wie copy & paste fehler aus.

@HomeAutoUser @elektron-bbs
Könnte ich das ggf. mal Anhand von Stichproben mit verifizieren?

@HomeAutoUser
Copy link
Contributor

HomeAutoUser commented Feb 9, 2021

@sidey79,

Da sind ein paar Tests, da stimmen die Werte aus dem JSON nicht.

zu 10_SD_GT
Die Daten kannst du nicht so mit dem Test prüfen wie die Anderen. Grund dafür ist, das die Daten im JSON schon richtig sind

Bsp:
MU;P0=328;P1=-428;P3=1090;P4=-1190;P5=-2310;D=010131040431310431043131313131043104313104040531310404310404313104310431313131310431043131040405313104043104043131043104313131313104310431310404053131040431040431310431043131313131043104313104042;CP=0;

legt ein Device SD_GT_LEARN an mit den Readings

Name Value
LearnCodes C9AFAC
state learned code 1, please press another button

an WENN die rmsg als Erstes empfangen wird. Da das Modul von einem Device erstmal Codes sammelt um dann in einem Hash nach der Variante zu schauen, so kommt bei der nächsten rmsg dann die nächste DMSG unter dem selben Device SD_GT_LEARN an aber das Reading LearnCodes erweitert sich um den Wert.

Bsp mit der nächsten rmsg:
MU;P0=1090;P1=-430;P2=328;P3=-1192;P4=-2319;P5=2988;D=01012323232301232323012301230101012323010101232324010123232323012323230123012301010123230101012323240101232323230123232301230123010101232301010123232401012323232301232323012301230101012323010101232353;CP=2;
wird dann

Name Value
LearnCodes C9AFAC,C22B9C
state learned code 2, please press another button

usw.... bis die Anzahl von glaube 6 erreicht ist und dann erfolgt die Prüfung im Hash ob es das Device gibt. Das ist so vorgegeben.

!!! Das du die selben Daten wie im JSON erhälst, müsstest du nach jeder rmsg welche dispatched wurde, die Readings löschen als ob es immer die Erstanlegung wäre. So würdest du die Werte wie im JSON enthalten !!!

zu 14_SD_UT
Die Daten im JSON sind richtig. Dein Vorgehen beim Test ist aber falsch. Du kannst hier ebenso nicht nur die rmsg dispatchen um auf das Ergebnis aus dem JSON zu kommen. Du musst nach dem Anlegen des Devices unknown_please_select_model mit den Readings

Name Value
state ???
unknownMSG 011000000100 (protocol: 81) <--- das ist ein Bsp

das Attribut des Modelles auswählen und setzen. Nach erneutem Dispatch deiner rmsg kommt dann das richtige Ergebnis des Devices. Der Test müsste analog wie https://github.com/RFD-FHEM/RFFHEM/blob/master/UnitTest/tests/test_09_SD_UT_devices-definition.txt ablaufen und nicht einfach nur die rmsg parsen ;-)

zu 14_SD_WS
Die Daten im JSON sind richtig.
Im JSON sind unter einem Eintrag jeweils nur unterschiedliche rmsg´s welche zu unterschiedlichen Readings führen. Du musst also auch hier nach jeder Prüfung das Device bzw. alle Readings löschen sonst kommt es zu Fehlern.

Bsp.:

2021-02-08T22:13:21.3004085Z             # +--------------+----+--------------+
2021-02-08T22:13:21.3004587Z             # | GOT          | OP | CHECK        |
2021-02-08T22:13:21.3005264Z             # +--------------+----+--------------+
2021-02-08T22:13:21.3005744Z             # | T: 3.8 H: 76 | eq | T: 5.3 H: 75 |
2021-02-08T22:13:21.3006398Z             # +--------------+----+--------------+

ist von diesem hier:

{"name":"EFTH-800", "id":"27", "module":"SD_WS", "data": [
    {
      "dmsg":"W27#113C49B04806", "comment":"EuroChron weatherstation EFTH-800 / Channel 2 (ID 61 additionally)", "user":"elektron-bbs",
      "internals": {"DEF":"SD_WS_27_TH_2", "NAME":"SD_WS_27_TH_2"},
      "readings": {"state":"T: 15.5 H: 48", "batteryState":"ok", "channel":"2", "humidity":"48", "temperature":"15.5", "type":"EFTH-800, EFS-3110A"},
      "rmsg":"MU;P0=-224;P1=258;P2=-487;P3=505;P4=-4884;P5=743;P6=-718;D=0121212301212303030301212123012123012123030123030121212121230121230121212121212121230301214565656561212123012121230121230303030121212301212301212303012303012121212123012123012121212121212123030121;CP=1;R=53;"
    },
    {
      "dmsg":"W27#21D442607679", "comment":"EuroChron weatherstation EFTH-800 / Channel 3 (ID 61 additionally)", "user":"elektron-bbs",
      "internals": {"DEF":"SD_WS_27_TH_3", "NAME":"SD_WS_27_TH_3"},
      "readings": {"state":"T: 3.8 H: 76", "batteryState":"ok", "channel":"3", "humidity":"76", "temperature":"3.8", "type":"EFTH-800, EFS-3110A"},
      "rmsg":"MU;P0=-241;P1=251;P2=-470;P3=500;P4=-4868;P5=743;P6=-718;D=012121212303030123012301212123012121212301212303012121212121230303012303012123030303012123014565656561212301212121230303012301230121212301212121230121230301212121212123030301230301212303030301212301;CP=1;R=23;"
    },
    {
      "dmsg":"W27#21D4435075DE", "comment":"EuroChron weatherstation EFTH-800 / Channel 3 (ID 61 additionally)", "user":"elektron-bbs",
      "internals": {"DEF":"SD_WS_27_TH_3", "NAME":"SD_WS_27_TH_3"},
      "readings": {"state":"T: 5.3 H: 75", "batteryState":"ok", "channel":"3", "humidity":"75", "temperature":"5.3", "type":"EFTH-800, EFS-3110A"},
      "rmsg":"MU;P0=-240;P1=253;P2=-487;P3=489;P4=-4860;P5=746;P6=-725;D=012121212303030123012301212123012121212303012301230121212121230303012301230303012303030301214565656561212301212121230303012301230121212301212121230301230123012121212123030301230123030301230303030121;CP=1;R=19;"
    }
  ]
},

zu 14_SD_WS09

... das wird analog wie SD_WS sein.

KURZGESAGT, du kannst nicht nur Parsen um dann das richtige Ergebnis zu erhalten! Der User muss auch mal noch etwas
machen aber das ist nicht anders möglich umzusetzen aufgrund der sehr ähnlichen Herstellerumsetzungen bzw. Chinaclones ;-)
Du kannst auch schauen ob im JSON der Wert Attributes ist und dann müssen erst diese GESETZT werden um das richtige Ergebnis zu erhalten.

VORSCHLAG, bitter erst bei Tests fragen @elektron-bbs oder @HomeAutoUser bevor du den Test falsch ansetzt obwohl die Vorgabe im JSON richtig ist smile

@sidey79
Copy link
Contributor Author

sidey79 commented Feb 9, 2021

zu 14_SD_UT
Die Daten im JSON sind richtig. Dein Vorgehen beim Test ist aber falsch. Du kannst hier ebenso nicht nur die rmsg dispatchen um auf das Ergebnis aus dem JSON zu kommen. Du musst nach dem Anlegen des Devices unknown_please_select_model mit den Readings

Name Value
state ???
unknownMSG 011000000100 (protocol: 81) <--- das ist ein Bsp
das Attribut des Modelles auswählen und setzen. Nach erneutem Dispatch deiner rmsg kommt dann das richtige Ergebnis des Devices. Der Test müsste analog wie https://github.com/RFD-FHEM/RFFHEM/blob/master/UnitTest/tests/test_09_SD_UT_devices-definition.txt ablaufen und nicht einfach nur die rmsg parsen ;-)

@HomeAutoUser
Erstmal vielen Dank, dass Du dir das so intensiv angesehen hast. Ich hatte angenommen, es wäre eine tolle Sache, wenn wir ein Testfall haben, den wir für jedes Modul ohne Anpassungen verwenden können.
Bei manchen Modulen klappt das ja auch, bei anderen halt noch nicht :)

Ich muss Da jetzt erst mal Stück für Stück durcharbeiten.
Bei SD_UT habe ich aber Schwierigkeiten dir zu Folgen.

  1. im Testfile wird keine rmsg dispatcht, sondern dmsg. Hast Du es nur verwechselt oder etwas anderes angesehen?
  2. Ich verstehe, dass ich das Modell setzen soll ok. Aber das haben wir im test_09_SD_UT_devices-definition.txt auch nicht machen müssen.
    Die Fehlermeldung deutet eher auf ein Problem beim define hin:
    2021.02.08 22:13:01 1: define BeSmart_S4_534 SD_UT BeSmart_S4 534: wrong <model> BeSmart_S4 und das def kommt ja aus dem https://github.com/RFD-FHEM/SIGNALduino_TOOL/blob/67013e96de0a18f8cbbd64d9259218122efaa568/FHEM/lib/SD_Device_ProtocolList.json#L1151
    .
    Und letztendlich denke ich, dass zumindest hier das Problem ist, dass im JSON was steht, was im Modul nicht angekommen ist.
    Remote control SEAV BeSmart S4 #933 ist einfach noch nicht in meine feature Branch enthalten.
    Das kann ich im Test aber nicht feststellen, ob im JSON was enthalten ist, das im Modul noch nicht existiert, einziger Workaround ist wieder auf das Vorhandensein der Protocol ID zu prüfen.

Wenn ich weitere Fragen habe, dann schreibe ich einen neuen Kommentar.

@sidey79
Copy link
Contributor Author

sidey79 commented Feb 9, 2021

@HomeAutoUser
In dem Tests habe ich am Ende jedes Subtests ein CommanDelete eingefügt. Damit wird dann das angelegte Gerät nach dem Test gelöscht und es existieren für Folgende Tests keine Leichen.

SD_WS und SD_WS09 laufen damit nun fehlerfrei durch.

SD_GT muss ich wohl ein paar Minuten mehr investieren :)

@sidey79
Copy link
Contributor Author

sidey79 commented Feb 9, 2021

@HomeAutoUser
Den Fall zu SD_GT habe ich bei dem Reading learncode vermutlich verstanden, dass sich das Reading erweitert wenn es das gerät schon gibt.
Da ich ja jetzt ein CommandDelete eingebaut habe, gibt es für jeden Testdatensatz immer ein neues, frisches Gerät.

Das Reading state weicht aber von dem im json angegebenen Stand ab. Ich nehme an, die JSON Angabe stimmt hier nicht. Korrekt?

  not ok 2 - Verify readings {
            1..2
            ok 1 - check reading LearnCodes
            not ok 2 - check reading state
            # Failed test 'check reading state'
            # at /mnt/z/Benutzer/Sven/Documents/eclipse workspace/cresta-fhem/RFFHEM/t/FHEM/10_SD_GT/09_parseDatat.t line 113.
            # +---------------------------+----+---------------------------+
            # | GOT                       | OP | CHECK                     |
            # +---------------------------+----+---------------------------+
            # | learned code 1, please pr | eq | code already registered,  |
            # | ess another button        |    | please press another butt |
            # |                           |    | on                        |
            # +---------------------------+----+---------------------------+
        # Failed test 'Verify readings'
        # at /mnt/z/Benutzer/Sven/Documents/eclipse workspace/cresta-fhem/RFFHEM/t/FHEM/10_SD_GT/09_parseDatat.t line 115.

@sidey79
Copy link
Contributor Author

sidey79 commented Feb 12, 2021

Sieht leider wie ein riesiger PR aus. War aber leider nicht kleiner machbar :(

Im wesentlichen habe ich nur alle Tests in ein Standard Perl Format migriert und dabei Korrigiert, dass das Modul auch ohne fhemweb läuft.
Den Test für das RSL Modul habe ich gestrichen. Das Modul werde ich hier löschen, da es im Repo fhem/mod_rsl liegt.

@sidey79 sidey79 merged commit 5556b02 into master Feb 13, 2021
@sidey79 sidey79 deleted the unittest-2-prove branch February 13, 2021 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants