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

testData files update #1155

Merged
merged 15 commits into from
Feb 20, 2023
Merged

testData files update #1155

merged 15 commits into from
Feb 20, 2023

Conversation

HomeAutoUser
Copy link
Contributor

  • 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)
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
  • What is the current behavior?
    (You can also link to an open issue here, if this describes the current behavior)

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

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

  • Other information:
    -- all data sets have a uniform structure

41_OREGON - removal of unnecessary details
14_SD_WS_Maverick - removal of unnecessary details
14_SD_WS09 - removal of unnecessary details
14_SD_WS07 - removal of unnecessary details
14_SD_WS - removal of unnecessary details and fix testdata #1149 (comment)
14_SD_UT - removal of unnecessary details
14_SD_BELL - removal of unnecessary details
14_SD_AS - removal of unnecessary details
14_Hideki - removal of unnecessary details
14_FLAMINGO - removal of unnecessary details
10_SD_Rojaflex - removal of unnecessary details
10_SD_GT - removal of unnecessary details
10_FS10 - removal of unnecessary details
@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Merging #1155 (7524b3b) into master (e17afb9) will increase coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1155      +/-   ##
==========================================
+ Coverage   67.53%   67.62%   +0.09%     
==========================================
  Files         133      136       +3     
  Lines        9802     9817      +15     
  Branches     1570     1570              
==========================================
+ Hits         6620     6639      +19     
+ Misses       1887     1883       -4     
  Partials     1295     1295              
Flag Coverage Δ
fhem 57.26% <ø> (+0.12%) ⬆️
modules 67.62% <ø> (+0.09%) ⬆️
perl 90.33% <ø> (+0.16%) ⬆️
unittests 67.62% <ø> (+0.09%) ⬆️

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

Impacted Files Coverage Δ
lib/Test2/SIGNALduino/RDmsg.pm 73.00% <0.00%> (-1.00%) ⬇️
t/FHEM/14_SD_UT/00_load.t 100.00% <0.00%> (ø)
t/FHEM/14_SD_AS/00_load.t 100.00% <0.00%> (ø)
t/FHEM/10_SD_GT/00_load.t 100.00% <0.00%> (ø)
FHEM/10_SD_Rojaflex.pm 71.25% <0.00%> (+0.80%) ⬆️
FHEM/14_Hideki.pm 74.19% <0.00%> (+1.61%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sidey79
Copy link
Contributor

sidey79 commented Jan 30, 2023

Die Schreibweise in den Kommentaren waren und sind noch sehr unterschiedlich, das ist mir jetzt aufgefallen.
Ich würde vorschlagen wir fangen an die Abschnitte in tests [] bei #1 beginnend durchzunummerieren.
Meist gibt es ja nur einen Testfall im Datensatz.

@HomeAutoUser
Copy link
Contributor Author

HomeAutoUser commented Jan 31, 2023

Die Schreibweise in den Kommentaren waren und sind noch sehr unterschiedlich, das ist mir jetzt aufgefallen.

Ich finde es sehr lustig, das dir selbst erst manche Dinge auffallen, wenn geballt von uns etwas eingereicht wird :-)
Du hattest diese Tests erstellt.

Ich würde vorschlagen wir fangen an die Abschnitte in tests [] bei #1 beginnend durchzunummerieren.
Meist gibt es ja nur einen Testfall im Datensatz.

Wie meinst du dies?

Ich für meine Automatik habe bei comment immer mit der 0 im ersten Datensatz angefangen zu zählen.
Was ich natürlich nicht gemacht habe bzw. machen haben lasse, wenn du dort einen Kommentar schon drin hattest, (Bsp: "Test #1" oder "#1" diesen zu ändern. Ja, da gibt es nun unterschiedliche Schreibweisen. Das ist aber auch dem geschuldet, das TestFiles teilweise ohne Kommentar versehen waren und ich somit eine Vereinheitlichung bringen wollte.

Grund für die Vereinheitlichung liegt darin, das ich somit selbst generierte TestData Files besser vergleichen kann ;-)

Kurz gesagt, die farblichen Markierungen sind auch zum Großteil wieder eine "optische Täuschung". Um das richtig zu "bewerten" müsstest du im Branch https://github.com/RFD-FHEM/RFFHEM/tree/master_t_rev/t/FHEM sämtliche von mir angetasteten Dateien manuell ansehen.

@sidey79
Copy link
Contributor

sidey79 commented Feb 5, 2023

Die Schreibweise in den Kommentaren waren und sind noch sehr unterschiedlich, das ist mir jetzt aufgefallen.

Ich finde es sehr lustig, das dir selbst erst manche Dinge auffallen, wenn geballt von uns etwas eingereicht wird :-) Du hattest diese Tests erstellt.

:) Stimmt. Manche habe ich durch ein Script konvertiert, manche händisch gemacht. Daher sind sie nicht wirklich einheitlich.
Da war es mir wichtiger, dass die Tests überhaupt mal wieder laufen.

Ich würde vorschlagen wir fangen an die Abschnitte in tests [] bei #1 beginnend durchzunummerieren.
Meist gibt es ja nur einen Testfall im Datensatz.

Wie meinst du dies?

Ich meine das so, tests selbst enhält eine Liste [] von Objekten {}.

            "tests" : [
               {
                  "comment" : "#3"

Jedes Objekt hat ein eigenes Kommentar.
Wir sollten einheitlich bei 1 anfangen und nicht bei 3 oder 0.

Ich für meine Automatik habe bei comment immer mit der 0 im ersten Datensatz angefangen zu zählen.

0 Ginge natürlich auch für den ersten Test.

Kurz gesagt, die farblichen Markierungen sind auch zum Großteil wieder eine "optische Täuschung". Um das richtig zu "bewerten" müsstest du im Branch https://github.com/RFD-FHEM/RFFHEM/tree/master_t_rev/t/FHEM sämtliche von mir angetasteten Dateien manuell ansehen.

Ich dachte das habe ich, aber mag sein, dass ich mich hier habe täuschen lassen. Das erlärt ja auch, weshalb die Tests ohne Fehler durchlaufen.

Beim Löschen der user Referenz bin ich mir nicht so sicher ob uns das hilft. Kürzlich fand ich es doch hilfreich, einen User zu den Testdaten befragen zu können. Wollen wir die vielleicht doch drinnen lassen?

@HomeAutoUser
Copy link
Contributor Author

Da bin ich wieder an der "Rechenmaschine" :-D

@sidey79

0 Ginge natürlich auch für den ersten Test.

Soll ich die vorhandenen Kommentare, welche nicht einer "Nummer" entsprechen anpassen bzw. anpassen lassen, so das wir eine fortlaufende Zahl von 0 (für den ersten Test) aufsteigend durchzählen?
"comment" : "#0" ... bis Bsp.: "comment" : "#5" bei 6 Tests.

Beim Löschen der user Referenz bin ich mir nicht so sicher ob uns das hilft. Kürzlich fand ich es doch hilfreich, einen User zu den Testdaten befragen zu können. Wollen wir die vielleicht doch drinnen lassen?

Das sind keine Informationen welche in den Testdaten sein müssen. Diese Informationen sind in dem JSON von uns enthalten. Sobald Daten von uns zu dir in die Testdaten kommen, werden automatisch im JSON schon die Daten verankert. Aus diesem Grund habe ich diese aus den Testdaten entfernt. Wenn du also den User wissen möchtest, musst du nur im JSON vom TOOL nachsehen ;-)

@sidey79
Copy link
Contributor

sidey79 commented Feb 14, 2023

Da bin ich wieder an der "Rechenmaschine" :-D

@sidey79

0 Ginge natürlich auch für den ersten Test.

Soll ich die vorhandenen Kommentare, welche nicht einer "Nummer" entsprechen anpassen bzw. anpassen lassen, so das wir eine fortlaufende Zahl von 0 (für den ersten Test) aufsteigend durchzählen? "comment" : "#0" ... bis Bsp.: "comment" : "#5" bei 6 Tests.

Das wäre spitzte, wobei es mir egal ist ob wir bei 0 oder 1 zu zählen beginnen, wir sollten es nur einheitlich machen :)

Beim Löschen der user Referenz bin ich mir nicht so sicher ob uns das hilft. Kürzlich fand ich es doch hilfreich, einen User zu den Testdaten befragen zu können. Wollen wir die vielleicht doch drinnen lassen?

Das sind keine Informationen welche in den Testdaten sein müssen. Diese Informationen sind in dem JSON von uns enthalten. Sobald Daten von uns zu dir in die Testdaten kommen, werden automatisch im JSON schon die Daten verankert. Aus diesem Grund habe ich diese aus den Testdaten entfernt. Wenn du also den User wissen möchtest, musst du nur im JSON vom TOOL nachsehen ;-)

Es stimmt, dass sie in den Tests nicht sein müssen. Wie kann ich den Datensatz aus Tests mit dem aus TOOL in Verbindung bringen?

@HomeAutoUser
Copy link
Contributor Author

Das wäre spitzte, wobei es mir egal ist ob wir bei 0 oder 1 zu zählen beginnen, wir sollten es nur einheitlich machen :)

Das habe ich erledigt :-) und wir fangen bei 0 an.

Es stimmt, dass sie in den Tests nicht sein müssen. Wie kann ich den Datensatz aus Tests mit dem aus TOOL in Verbindung bringen?

Verstehe ich deine Frage richtig? Wie du den Datensatz aus den Tests im TOOL wiederfindest?
Alle bisherigen Tests wurden aus den Daten vom TOOL erstellt und so müsstest du nur die DMSG / RAWMSG und zur Kontrolle die ID.

Da die bisherige Entwicklung immer erst Daten in das TOOL brachte und dann für dich die Testdaten zur Verfügung gestellt / erstellt wurden, so baut dies nacheiner auf. TOOL Datensatz ---> wandert in die ---> Tests

Wenn nichts mehr offen ist, wäre ich von meiner Seite mit dem PR durch.

@HomeAutoUser
Copy link
Contributor Author

@sidey79 ich würde ja gern die Überführung vornehmen.

Unresolved conversations
13 conversations must be resolved before merging.

Merging is blocked
The base branch requires all conversations on code to be resolved. 

Das heißt? Wie können wir dies Squash an mergen?
Sind es vielleicht deine Kommentierungen welche du "abhaken" musst?

@sidey79
Copy link
Contributor

sidey79 commented Feb 20, 2023

Ich hoffe es sind dann alle resolved :)

@HomeAutoUser HomeAutoUser merged commit c3b7190 into master Feb 20, 2023
@elektron-bbs elektron-bbs deleted the master_t_rev branch June 19, 2024 15:57
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.

2 participants