-
Notifications
You must be signed in to change notification settings - Fork 170
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
[AI Test Tool] Add Multi-Turn Accuracy & Export #2419
base: main
Are you sure you want to change the base?
Conversation
/// <summary> | ||
/// Gets the accuracy of the test. | ||
/// </summary> | ||
/// <returns>True if the accuracy was set, false otherwise.</returns> |
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.
missing parameter doc for AccuracyPct
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.
It is not clear that this function returns accuracy only when it is being set manually.
@@ -155,7 +185,7 @@ codeunit 149043 "AIT Test Context Impl." | |||
if not IsMultiTurn then | |||
exit(false); | |||
|
|||
if CurrentTurn + 1 > NumberOfTurns then |
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.
This change may cause regression. Have you tested against some of our existing multi-turn scenarios? Just to be 100% sure.
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 can see CurrentTurn is now starts with 1.
@@ -68,6 +68,23 @@ page 149033 "AIT Log Entries" | |||
{ | |||
StyleExpr = StatusStyleExpr; | |||
} | |||
field(Accuracy; Rec.Accuracy) |
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.
we need to expose these in the API as well
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.
Feel free to create a separate work item, we can uptake the API changes in the same work item.
{ | ||
Caption = 'Export Results'; | ||
Image = Export; | ||
ToolTip = 'Exports the results.'; |
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.
Can you provide a better tool tip. It might be confusing with the existing action of download test output.
field(45; Accuracy; Decimal) | ||
{ | ||
Caption = 'Accuracy'; | ||
ToolTip = 'Specifies the accuracy of the test line.'; |
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.
nit: consider describing how the accuracy can be calculated or what does it exactly mean. Considering we have a status field as well.
ApplicationArea = All; | ||
UsageCategory = Tasks; | ||
DefaultLayout = Excel; | ||
ExcelLayout = 'Results.xlsx'; |
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.
consider using a better name for the layout
} | ||
field(TurnsText; TurnsText) | ||
{ | ||
Visible = 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.
Can we have this visible?
field(45; Accuracy; Decimal) | ||
{ | ||
Caption = 'Accuracy'; | ||
ToolTip = 'Specifies the average accuracy of the test line.'; |
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.
Should we explain in the tooltip how the accuracy is calculated? As it will not be a straightforward calculation of Sum(Numer of succsful turn)/Sum(number of turns)
ResultsInStream: InStream; | ||
Filename: Text; | ||
begin | ||
Filename := ResultsFileNameLbl; |
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.
nit: consider using the file name based on the suitename_version_result.xlsx
Summary
This PR adds multi-turn accuracy for tests and export option for results.
Accuracy is automatically calculated as the number of turns succeeded / the total number of turns. Accuracy is available on Suite level, codeunit level and test level.
Accuracy can also be set by the test manually, using the new SetAccuracy function in the AIT Test Context.
Export of results is available on the Log Entries page (exports current view) or Suite (exports results for latest version)
Work Item(s)
Fixes AB#557118