Skip to content

Commit 5124b2e

Browse files
alavissAraq
authored andcommitted
testament/azure: major rewrite (#13246)
This commit features a major rewrite of Azure Pipelines integration, turning the spaghetti it originally was into something maintainable. Key changes: - No longer requires a ton of hooks into testament. - Results are now cached then bulk-uploaded to prevent throttling from Azure Pipelines, avoiding costly timeouts. - A low timeout is also employed to avoid inflated test time. - The integration is now documented.
1 parent 981ffc9 commit 5124b2e

File tree

2 files changed

+125
-77
lines changed

2 files changed

+125
-77
lines changed

testament/azure.nim

Lines changed: 119 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -6,90 +6,142 @@
66
# Look at license.txt for more info.
77
# All rights reserved.
88

9-
import base64, json, httpclient, os, strutils
9+
import base64, json, httpclient, os, strutils, uri
1010
import specs
1111

1212
const
13-
ApiRuns = "/_apis/test/runs"
14-
ApiVersion = "?api-version=5.0"
15-
ApiResults = ApiRuns & "/$1/results"
16-
17-
var runId* = -1
13+
RunIdEnv = "TESTAMENT_AZURE_RUN_ID"
14+
CacheSize = 8 # How many results should be cached before uploading to
15+
# Azure Pipelines. This prevents throttling that might arise.
1816

1917
proc getAzureEnv(env: string): string =
2018
# Conversion rule at:
2119
# https://docs.microsoft.com/en-us/azure/devops/pipelines/process/variables#set-variables-in-pipeline
2220
env.toUpperAscii().replace('.', '_').getEnv
2321

24-
proc invokeRest(httpMethod: HttpMethod; api: string; body = ""): Response =
25-
let http = newHttpClient()
26-
defer: close http
27-
result = http.request(getAzureEnv("System.TeamFoundationCollectionUri") &
28-
getAzureEnv("System.TeamProjectId") & api & ApiVersion,
29-
httpMethod,
30-
$body,
31-
newHttpHeaders {
32-
"Accept": "application/json",
33-
"Authorization": "Basic " & encode(':' & getAzureEnv("System.AccessToken")),
34-
"Content-Type": "application/json"
35-
})
36-
if not result.code.is2xx:
37-
raise newException(HttpRequestError, "Server returned: " & result.body)
38-
39-
proc finish*() {.noconv.} =
40-
if not isAzure or runId < 0:
41-
return
22+
template getRun(): string =
23+
## Get the test run attached to this instance
24+
getEnv(RunIdEnv)
4225

43-
try:
44-
discard invokeRest(HttpPatch,
45-
ApiRuns & "/" & $runId,
46-
$ %* { "state": "Completed" })
47-
except:
48-
stderr.writeLine "##vso[task.logissue type=warning;]Unable to finalize Azure backend"
49-
stderr.writeLine getCurrentExceptionMsg()
26+
template setRun(id: string) =
27+
## Attach a test run to this instance and its future children
28+
putEnv(RunIdEnv, id)
5029

51-
runId = -1
30+
template delRun() =
31+
## Unattach the test run associtated with this instance and its future children
32+
delEnv(RunIdEnv)
5233

53-
# TODO: Only obtain a run id if tests are run
54-
# NOTE: We can't delete test runs with Azure's access token
55-
proc start*() =
56-
if not isAzure:
57-
return
58-
try:
59-
if runId < 0:
60-
runId = invokeRest(HttpPost,
61-
ApiRuns,
62-
$ %* {
63-
"automated": true,
64-
"build": { "id": getAzureEnv("Build.BuildId") },
65-
"buildPlatform": hostCPU,
66-
"controller": "nim-testament",
67-
"name": getAzureEnv("Agent.JobName")
68-
}).body.parseJson["id"].getInt(-1)
69-
except:
70-
stderr.writeLine "##vso[task.logissue type=warning;]Unable to initialize Azure backend"
71-
stderr.writeLine getCurrentExceptionMsg()
34+
template warning(args: varargs[untyped]) =
35+
## Add a warning to the current task
36+
stderr.writeLine "##vso[task.logissue type=warning;]", args
37+
38+
let
39+
ownRun = not existsEnv RunIdEnv
40+
## Whether the test run is owned by this instance
41+
accessToken = getAzureEnv("System.AccessToken")
42+
## Access token to Azure Pipelines
43+
44+
var
45+
active = false ## Whether the backend should be activated
46+
requestBase: Uri ## Base URI for all API requests
47+
requestHeaders: HttpHeaders ## Headers required for all API requests
48+
results: JsonNode ## A cache for test results before uploading
49+
50+
proc request(api: string, httpMethod: HttpMethod, body = ""): Response {.inline.} =
51+
let client = newHttpClient(timeout = 3000)
52+
defer: close client
53+
result = client.request($(requestBase / api), httpMethod, body, requestHeaders)
54+
if result.code != Http200:
55+
raise newException(CatchableError, "Request failed")
56+
57+
proc init*() =
58+
## Initialize the Azure Pipelines backend.
59+
##
60+
## If an access token is provided and no test run is associated with the
61+
## current instance, this proc will create a test run named after the current
62+
## Azure Pipelines' job name, then associate it to the current testament
63+
## instance and its future children. Should this fail, the backend will be
64+
## disabled.
65+
if isAzure and accessToken.len > 0:
66+
active = true
67+
requestBase = parseUri(getAzureEnv("System.TeamFoundationCollectionUri")) /
68+
getAzureEnv("System.TeamProjectId") / "_apis" ? {"api-version": "5.0"}
69+
requestHeaders = newHttpHeaders {
70+
"Accept": "application/json",
71+
"Authorization": "Basic " & encode(':' & accessToken),
72+
"Content-Type": "application/json"
73+
}
74+
results = newJArray()
75+
if ownRun:
76+
try:
77+
let resp = request(
78+
"test/runs",
79+
HttpPost,
80+
$ %* {
81+
"automated": true,
82+
"build": { "id": getAzureEnv("Build.BuildId") },
83+
"buildPlatform": hostCPU,
84+
"controller": "nim-testament",
85+
"name": getAzureEnv("Agent.JobName")
86+
}
87+
)
88+
setRun $resp.body.parseJson["id"].getInt
89+
except:
90+
warning "Couldn't create test run for Azure Pipelines integration"
91+
# Set run id to empty to prevent child processes from trying to request
92+
# for yet another test run id, which wouldn't be shared with other
93+
# instances.
94+
setRun ""
95+
active = false
96+
elif getRun().len == 0:
97+
# Disable integration if there aren't any valid test run id
98+
active = false
99+
100+
proc uploadAndClear() =
101+
## Upload test results from cache to Azure Pipelines. Then clear the cache
102+
## after.
103+
if results.len > 0:
104+
try:
105+
discard request("test/runs/" & getRun() & "/results", HttpPost, $results)
106+
except:
107+
for i in results:
108+
warning "Couldn't log test result to Azure Pipelines: ",
109+
i["automatedTestName"], ", outcome: ", i["outcome"]
110+
results = newJArray()
111+
112+
proc finalize*() {.noconv.} =
113+
## Finalize the Azure Pipelines backend.
114+
##
115+
## If a test run has been associated and is owned by this instance, it will
116+
## be marked as complete.
117+
if active:
118+
if ownRun:
119+
uploadAndClear()
120+
try:
121+
discard request("test/runs/" & getRun(), HttpPatch,
122+
$ %* {"state": "Completed"})
123+
except:
124+
warning "Couldn't update test run ", getRun(), " on Azure Pipelines"
125+
delRun()
72126

73127
proc addTestResult*(name, category: string; durationInMs: int; errorMsg: string;
74128
outcome: TResultEnum) =
75-
if not isAzure or runId < 0:
129+
if not active:
76130
return
131+
77132
let outcome = case outcome
78133
of reSuccess: "Passed"
79134
of reDisabled, reJoined: "NotExecuted"
80135
else: "Failed"
81-
try:
82-
discard invokeRest(HttpPost,
83-
ApiResults % [$runId],
84-
$ %* [{
85-
"automatedTestName": name,
86-
"automatedTestStorage": category,
87-
"durationInMs": durationInMs,
88-
"errorMessage": errorMsg,
89-
"outcome": outcome,
90-
"testCaseTitle": name
91-
}])
92-
except:
93-
stderr.writeLine "##vso[task.logissue type=warning;]Unable to log test case: ",
94-
name, ", outcome: ", outcome
95-
stderr.writeLine getCurrentExceptionMsg()
136+
137+
results.add(%* {
138+
"automatedTestName": name,
139+
"automatedTestStorage": category,
140+
"durationInMs": durationInMs,
141+
"errorMessage": errorMsg,
142+
"outcome": outcome,
143+
"testCaseTitle": name
144+
})
145+
146+
if results.len > CacheSize:
147+
uploadAndClear()

testament/testament.nim

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ Options:
4747
--backendLogging:on|off Disable or enable backend logging. By default turned on.
4848
--megatest:on|off Enable or disable megatest. Default is on.
4949
--skipFrom:file Read tests to skip from `file` - one test per line, # comments ignored
50+
51+
On Azure Pipelines, testament will also publish test results via Azure Pipelines' Test Management API
52+
provided that System.AccessToken is made available via the environment variable SYSTEM_ACCESSTOKEN.
5053
""" % resultsFile
5154

5255
type
@@ -606,6 +609,7 @@ proc main() =
606609
os.putEnv "NIMTEST_COLOR", "never"
607610
os.putEnv "NIMTEST_OUTPUT_LVL", "PRINT_FAILURES"
608611

612+
azure.init()
609613
backend.open()
610614
var optPrintResults = false
611615
var optFailing = false
@@ -656,8 +660,6 @@ proc main() =
656660
quit Usage
657661
of "skipfrom":
658662
skipFrom = p.val.string
659-
of "azurerunid":
660-
runId = p.val.parseInt
661663
else:
662664
quit Usage
663665
p.next()
@@ -670,7 +672,6 @@ proc main() =
670672
of "all":
671673
#processCategory(r, Category"megatest", p.cmdLineRest.string, testsDir, runJoinableTests = false)
672674

673-
azure.start()
674675
var myself = quoteShell(findExe("testament" / "testament"))
675676
if targetsStr.len > 0:
676677
myself &= " " & quoteShell("--targets:" & targetsStr)
@@ -679,8 +680,6 @@ proc main() =
679680

680681
if skipFrom.len > 0:
681682
myself &= " " & quoteShell("--skipFrom:" & skipFrom)
682-
if isAzure:
683-
myself &= " " & quoteShell("--azureRunId:" & $runId)
684683

685684
var cats: seq[string]
686685
let rest = if p.cmdLineRest.string.len > 0: " " & p.cmdLineRest.string else: ""
@@ -706,16 +705,14 @@ proc main() =
706705
progressStatus(i)
707706
processCategory(r, Category(cati), p.cmdLineRest.string, testsDir, runJoinableTests = false)
708707
else:
709-
addQuitProc azure.finish
708+
addQuitProc azure.finalize
710709
quit osproc.execProcesses(cmds, {poEchoCmd, poStdErrToStdOut, poUsePath, poParentStreams}, beforeRunEvent = progressStatus)
711710
of "c", "cat", "category":
712-
azure.start()
713711
skips = loadSkipFrom(skipFrom)
714712
var cat = Category(p.key)
715713
p.next
716714
processCategory(r, cat, p.cmdLineRest.string, testsDir, runJoinableTests = true)
717715
of "pcat":
718-
azure.start()
719716
skips = loadSkipFrom(skipFrom)
720717
# 'pcat' is used for running a category in parallel. Currently the only
721718
# difference is that we don't want to run joinable tests here as they
@@ -730,7 +727,6 @@ proc main() =
730727
p.next
731728
processPattern(r, pattern, p.cmdLineRest.string, simulate)
732729
of "r", "run":
733-
azure.start()
734730
# at least one directory is required in the path, to use as a category name
735731
let pathParts = split(p.key.string, {DirSep, AltSep})
736732
# "stdlib/nre/captures.nim" -> "stdlib" + "nre/captures.nim"
@@ -745,8 +741,8 @@ proc main() =
745741
if optPrintResults:
746742
if action == "html": openDefaultBrowser(resultsFile)
747743
else: echo r, r.data
744+
azure.finalize()
748745
backend.close()
749-
if isMainProcess: azure.finish()
750746
var failed = r.total - r.passed - r.skipped
751747
if failed != 0:
752748
echo "FAILURE! total: ", r.total, " passed: ", r.passed, " skipped: ",

0 commit comments

Comments
 (0)