Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-50087

Integration Test for Table Model (in LabelProvider)

        [JENKINS-50087] Integration Test for Table Model (in LabelProvider)

        Außerdem scheint auch die Eingabe verschiedener Werte für die Thresholds nicht beachtet zu werden. Es werden scheinbar immer die Default-Werte von 50 und 25 genommen, egal was man auch eingibt. Bin mir zwar gerade nicht sicher, ob die beiden Probleme zusammen hängen, aber es scheint mir nicht so. Da die Werte ja gespeichert werden, auch wenn die Ajax-Validierung von Jenkins fehl schlägt.

        Stephan Plöderl added a comment - Außerdem scheint auch die Eingabe verschiedener Werte für die Thresholds nicht beachtet zu werden. Es werden scheinbar immer die Default-Werte von 50 und 25 genommen, egal was man auch eingibt. Bin mir zwar gerade nicht sicher, ob die beiden Probleme zusammen hängen, aber es scheint mir nicht so. Da die Werte ja gespeichert werden, auch wenn die Ajax-Validierung von Jenkins fehl schlägt.

        Ulli Hafner added a comment -

        Zum ersten Punkt: das habe ich vergessen zu sagen. Da habe ich auch schon mal den Debugger angeworfen und nichts gefunden. (Siehe JENKINS-50355)

        Ulli Hafner added a comment - Zum ersten Punkt: das habe ich vergessen zu sagen. Da habe ich auch schon mal den Debugger angeworfen und nichts gefunden. (Siehe  JENKINS-50355 )

        Ulli Hafner added a comment -

        Der zweite Punkt ist wahrscheinlich unabhängig und ein Bug!

        Ulli Hafner added a comment - Der zweite Punkt ist wahrscheinlich unabhängig und ein Bug!

        Habe in analysis-core-plugin im Pull-Request #122 ein paar Probleme behoben, die diesen Test behindert haben.

        Zum Beispiel das Problem, dass die Tabelle nicht geladen wird, wenn man die Seite aufruft.

        Dies passierte, weil die Ajax-Funktion nicht aufgerufen wurde, wenn man die Homepage besucht hatte, ohne einmal den Tab gewechselt zu haben.

        Sprich, wenn der activeTab im localStorage noch nicht gesetzt war.

        Ich versuche auch noch den Bug zu finden, welcher verhindert, dass die richtigen Thresholds genutzt werden.

        Stephan Plöderl added a comment - Habe in analysis-core-plugin im Pull-Request #122 ein paar Probleme behoben, die diesen Test behindert haben. Zum Beispiel das Problem, dass die Tabelle nicht geladen wird, wenn man die Seite aufruft. Dies passierte, weil die Ajax-Funktion nicht aufgerufen wurde, wenn man die Homepage besucht hatte, ohne einmal den Tab gewechselt zu haben. Sprich, wenn der activeTab im localStorage noch nicht gesetzt war. Ich versuche auch noch den Bug zu finden, welcher verhindert, dass die richtigen Thresholds genutzt werden.

        Ok, hab das Problem gefunden und sowohl für CPD, als auch für Simian behoben.

        Jetzt ist mir gerade aufgefallen, dass der Text für HighThreshold besagt, dass es die minimale Anzahl an Zeilen ist um ein High-Issue zu erzeugen.

        Allerdings ist der Code gerade so geschrieben, dass erst ab einem wert höher ein High issue erzeugt wird. (Das selbe gilt für normal)

        Jetzt die Frage, was soll jetzt genau richtig sein?

        ist es die minimale anzahl für ein high-issue oder die maximale anzahl um noch ein Normal-Issue zu erhalten?

        Stephan Plöderl added a comment - Ok, hab das Problem gefunden und sowohl für CPD, als auch für Simian behoben. Jetzt ist mir gerade aufgefallen, dass der Text für HighThreshold besagt, dass es die minimale Anzahl an Zeilen ist um ein High-Issue zu erzeugen. Allerdings ist der Code gerade so geschrieben, dass erst ab einem wert höher ein High issue erzeugt wird. (Das selbe gilt für normal) Jetzt die Frage, was soll jetzt genau richtig sein? ist es die minimale anzahl für ein high-issue oder die maximale anzahl um noch ein Normal-Issue zu erhalten?

        Ulli Hafner added a comment -

        Den ersten Bug habe ich inzwischen auch gefunden und behoben. (Noch nicht im hm-edu-branch).

        Ulli Hafner added a comment - Den ersten Bug habe ich inzwischen auch gefunden und behoben. (Noch nicht im hm-edu-branch).

        Ulli Hafner added a comment -

        Zum zweiten Bug: welche Variante ist eigentlich egal. Ich tippe mal dass der Code auch durch einen Test abgedeckt ist, daher macht es wohl eher Sinn, den Text anzupassen.

        Ulli Hafner added a comment - Zum zweiten Bug: welche Variante ist eigentlich egal. Ich tippe mal dass der Code auch durch einen Test abgedeckt ist, daher macht es wohl eher Sinn, den Text anzupassen.

        Hab das Problem gefunden. Es liegt an der Json-Generierung von den Duplicate-code warnings.

        File: DuplicateCodeScanner.java

        Dort wird in der toJson-Methode die Zeilenanzahl falsch berechnet:

                @Override
                protected JSONArray toJson(final Issue issue, final AgeBuilder ageBuilder) {
                    JSONArray columns = new JSONArray();
                    columns.add(formatDetails(issue));
                    columns.add(formatFileName(issue));
                    columns.add(formatPriority(issue.getPriority()));
                    columns.add(issue.getLineEnd() - issue.getLineStart() + 1);
                    columns.add(formatTargets(issue));
                    columns.add(formatAge(issue, ageBuilder));
                    return columns;
                }
        

        Das +1 ist zu viel, weshalb die Zeilenanzahl falsch angezeigt wird.

        Und da die Zeilenanzahl im Codeparser die wirkliche Zeilenanzahl, welche z.B. Cpd ausgibt verwendet, wird für die Priority zum Beispiel 3 doppelte Lines verwendet und in der Weboberfläche dann 4 angezeigt. Was zu einem Widerspruch zu den High- und NormalThresholds führt.
        Ich schau noch, ob die änderung irgendwelche auswirkungen auf die bisherigen tests hat.

        Überprüfe noch die Berechnung der doppelten Code Zeilen für Simian und stelle dann einen gesonderten Pull-Request.

        Stephan Plöderl added a comment - Hab das Problem gefunden. Es liegt an der Json-Generierung von den Duplicate-code warnings. File: DuplicateCodeScanner.java Dort wird in der toJson-Methode die Zeilenanzahl falsch berechnet:         @Override         protected JSONArray toJson( final Issue issue, final AgeBuilder ageBuilder) {             JSONArray columns = new JSONArray();             columns.add(formatDetails(issue));             columns.add(formatFileName(issue));             columns.add(formatPriority(issue.getPriority()));             columns.add(issue.getLineEnd() - issue.getLineStart() + 1);             columns.add(formatTargets(issue));             columns.add(formatAge(issue, ageBuilder));             return columns;         } Das +1 ist zu viel, weshalb die Zeilenanzahl falsch angezeigt wird. Und da die Zeilenanzahl im Codeparser die wirkliche Zeilenanzahl, welche z.B. Cpd ausgibt verwendet, wird für die Priority zum Beispiel 3 doppelte Lines verwendet und in der Weboberfläche dann 4 angezeigt. Was zu einem Widerspruch zu den High- und NormalThresholds führt. Ich schau noch, ob die änderung irgendwelche auswirkungen auf die bisherigen tests hat. Überprüfe noch die Berechnung der doppelten Code Zeilen für Simian und stelle dann einen gesonderten Pull-Request.

        oder besser gesagt schöner wäre es die endline berechnung in Cpd (vllt. auch simian, weiß ich noch nicht) zu fixen.

        Diese besteht nämlich aus startline + numberOfDuplicateLines.

        Stephan Plöderl added a comment - oder besser gesagt schöner wäre es die endline berechnung in Cpd (vllt. auch simian, weiß ich noch nicht) zu fixen. Diese besteht nämlich aus startline + numberOfDuplicateLines.

        fix ist in Pull-Request #66 in analysis-model

        Stephan Plöderl added a comment - fix ist in Pull-Request #66 in analysis-model

          stephan_p Stephan Plöderl
          drulli Ulli Hafner
          Votes:
          0 Vote for this issue
          Watchers:
          2 Start watching this issue

            Created:
            Updated:
            Resolved: