Reintroduce parallel exec 72658 #83
Conversation
lhinderberger
force-pushed
the
reintroduce-parallel-exec-72658
branch
2 times, most recently
from
c71b529 to
d4bbb2b
Compare
…ments This partially reverts commit 0a7debf.
With the new, simplified path syntax, we can replace the old path_utils with a simpler, more efficient new version.
Parallel Execution is re-introduced, but in a much simplified version. p@foo.json / pN@foo.json notation is gone, replaced by N@foo.json. When including a file with N@foo.json, its tests are still run serially, but the entire file is run in N parallel goroutines.
lhinderberger
force-pushed
the
reintroduce-parallel-exec-72658
branch
from
fdfdc46 to
261cde5
Compare
pkg/lib/util/file.go
Outdated
| // pN@ -> N parallel repetitions of tests | ||
| _, path = GetParallelPathSpec(path) | ||
| pathSpec, err := ParsePathSpec(path) | ||
| if err == nil { |
There was a problem hiding this comment.
Wir sollten den Error hier in "_" capturen und einen Comment reinschreiben dass, wenn die path spec falsch ist das schon vorher abgefangen wird.
There was a problem hiding this comment.
Wieso nicht als Rückfallebene drin lassen, falls weiter vorne mal nicht oder nicht richtig validiert wird?
There was a problem hiding this comment.
Mal davon abgesehen kann es sich bei der Eingabe ja auch um eine http-URL handeln. Das ist dann ohnehin nichts, was bei ParsePathSpec fehlerfrei durchgehen würde.
There was a problem hiding this comment.
Korrigiere, in der aktuellen Fassung würde es durchgehen.
There was a problem hiding this comment.
...außer es ist eine URL mit eingebauten Basic-Credentials (also sowas wie http://foo:bar@example.com/some/protected/resource) - denn dann würde es an strconv.Atoi scheitern
There was a problem hiding this comment.
ach können wir da eine URL verwenden? dann ist ParseSpec doch eh noch kaputt, weil eine URL mit creds als error verstanden wird oder? Der Punkt mit dem "err" zu "" ist, dass das "weiter oben" doch auf jeden Fall reported wird, deshalb sollte es hier nur ein "" sein (mit comment)
There was a problem hiding this comment.
Ich muss mich hier korrigieren: Die PathSpec ist natürlich nicht dafür gedacht, URLs zu beinhalten. Das musste ich gerade auf die harte Tour lernen, nachdem ich das dahingehend umgebaut habe (URLs in die PathSpec mit reingenommen, OpenFileOrUrl so umgebaut, dass es nur noch PathSpec entgegennimmt, rufende Funktionen nachgezogen), und dann feststellen musste dass die Tests fehlschlagen, weil ich hier zwei verschiedene Konzepte gedanklich vermischt habe: @-Notation und Einbindung beliebiger Dateien per Filename oder URL. Ich werde es jetzt genau so machen, wie du es in deinem ersten Kommentar vorgeschlagen hast.
There was a problem hiding this comment.
Ich sehe gerade, dass ein Capture in "_" nicht möglich ist, weil wir ja wissen müssen, ob es sich bei path um eine "@"-Notation PathSpec handelt (statt um einen "gewöhnlichen" Pfad). Ich habe aber einen erklärenden Kommentar ergänzt. Wir sollten uns außerdem Überlegen, ob wir PathSpec nicht vllt. umbenennen wollen um klar zu machen, dass es mit der "@"-Notation zu tun hat. Vllt. AtNotationPath?
By instruction of Martin, I removed Closer from build_policies and added warning comments to the code instead.
No description provided.