-
Notifications
You must be signed in to change notification settings - Fork 30
Conversation
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
…init Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
…path Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
Signed-off-by: Matt Oswalt <[email protected]>
I believe I've addressed your comments. |
log.Debugf("Full testlet command and args: '%s %s %s'", testlet_path, thisTarget, tr.Args) | ||
cmd := exec.Command(testlet_path, thisTarget, tr.Args) | ||
log.Debugf("Full testlet command and args: '%s %s %s'", testletPath, thisTarget, tr.Args) | ||
cmd := exec.Command(testletPath, thisTarget, tr.Args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General note, this could be changed to use exec.CommandContext
(added in 1.7) and simplify the timeout logic below.
ctx, cancel := context.WithTimeout(context.Background(), time.Duration(ett.TimeLimit) * time.Second)
defer cancel()
cmd := exec.CommandContext(ctx, testletPath, thisTarget, tr.Args)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created https://github.com/Mierdin/todd/issues/93 to track this so that when I move CI to Go 1.7, I'll add this. Thanks
|
||
// Generate path to testlet and make sure it exists. | ||
testletPath := fmt.Sprintf("%s/assets/testlets/%s", optDir, testletName) | ||
if _, err := os.Stat(testletPath); err != nil || os.IsNotExist(err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
os.IsNotExist(err)
will only be evaluated if err is nil, in which case os.IsNotExist
will always return false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, my intention was that regardless of the error, I wanted to treat it like the testlet didn't exist. However, that's fairly confusing.
I clarified this a bit by making the error more generic (and removing os.IsNotExist(err)
in Mierdin@6f1db24 - stating in the error that there was simply a problem accessing the testlet
Signed-off-by: Matt Oswalt <[email protected]>
This PR introduces native testlet capabilities - that is, the idea of providing functionality for the tests themselves, in native-go testlets, that are integrated at install time. A summary of changes are as follows
Testlet
interface) was maintained to encourage compatibility.ping
testlet, as that has been implemented here. The other bash testlets will be phased out in future PRs as their functionality is replaced by native Go testlets in the same way.gettestlets.sh
This also satisfies https://github.com/Mierdin/todd/issues/87