-
Notifications
You must be signed in to change notification settings - Fork 172
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
Marketing Text AI Tests Added #1504
base: main
Are you sure you want to change the base?
Conversation
…res/521281-entity-text-tests
"brief": "", | ||
"description": "Tests for Entity Text", | ||
"version": "25.0.0.0", | ||
"privacyStatement": "https://go.microsoft.com/fwlink/?LinkId=724011", |
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.
Are the links corect?
|
||
codeunit 134640 "Mkt Text Accuracy AIT" | ||
{ | ||
Subtype = Test; |
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.
Check if TestPermissions needs to be set to disabled.
|
||
codeunit 134641 "Mkt Text RedTeam AIT" | ||
{ | ||
Subtype = Test; |
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.
Check if TestPermissions needs to be set to disabled.
using System.TestTools.AITestToolkit; | ||
using System.Text; | ||
|
||
codeunit 134640 "Mkt Text Accuracy AIT" |
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 move the tests to a folder for the time being to distinguish between AIT Tests vs regular tests?
In the near future, we maybe introduce a separate codeunit number range.
The test app is also missing the Suite Configuration (.xml) file.
AITestContext.SetTestOutput(PrepareOutput(BuildFactsText(Facts, Category), Category, Response, Enum::"Entity Text Tone"::Inspiring, Format)); | ||
end; | ||
|
||
[Test] |
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.
Tests are missing comments:
[Scenario]
[Given]
[When]
[Then]
Did we stop using these?
local procedure PrepareOutput(Facts: Text; Response: Text; Tone: Enum "Entity Text Tone"; Format: Enum "Entity Text Format"): Text; | ||
var | ||
Context: Text; | ||
FormatLbl: Label '{"question": "METAPROMPT %1", "answer": "%2", "context": "%3", "ground_truth": "%4", "tone": "%5", "format" : "%6"}', Comment = '%1= Input Prompt, %2= Response Prompt, %3= Context, %4= Ground Truth, %5= Tone, %6= Format'; |
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 130462 "Test Output Json" instead of creating JSON as a string. Double quotes are not being escaped while setting the json property values.
begin | ||
NewLineChar := 10; | ||
Context := 'Here are some facts about the item:<br /><br />' + Facts.Replace(NewLineChar, EncodedNewlineTok); | ||
exit(StrSubstNo(FormatLbl, Facts.Replace(NewLineChar, EncodedNewlineTok), Response.Replace(NewLineChar, EncodedNewlineTok), Context, '', Tone.Names.Get(Tone.Ordinals.IndexOf(Tone.AsInteger())), Format.Names.Get(Format.Ordinals.IndexOf(Format.AsInteger())))); |
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 directly get the caption from the Tone enum?
InputPrompt: Text; | ||
Response: Text; | ||
begin | ||
InputJson.ReadFrom(AITestContext.GetInput().ValueAsText()); |
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.
The properties can be directly read using AITTestContext.GetInput() -> (returns Codeunit "Test Input Json")
AITestContext.GetInput().Element('inputPrompt').ValueAsText()
Can we use 'question' in the dataset instead of 'inputPrompt'? This would standardize the term usage across all the datasets.
And then you can also use AITestContext.GetQuestion().ValueAsText() to read the input.
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.
Comments are valid for both the test codeunits
NewLineChar: Char; | ||
begin | ||
NewLineChar := 10; | ||
Context := 'Here are some facts about the item:<br /><br />' + Facts.Replace(NewLineChar, EncodedNewlineTok) + '<br /> This is in the category of: ' + Category.Replace(NewLineChar, EncodedNewlineTok); |
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.
Labels? If this shouldn't be translated, shouldn't than FormatLbl be locked?
InputPrompt := AITestContext.GetInput().ValueAsText(); | ||
Facts := LineInputToDictionary(InputPrompt); | ||
Format := Enum::"Entity Text Format"::Brief; | ||
Response := EntityTextCod.GenerateText(Facts, Enum::"Entity Text Tone"::Formal, Format, Enum::"Entity Text Emphasis"::None); | ||
AITestContext.SetTestOutput(PrepareOutput(BuildFactsText(Facts, Category), Category, Response, Enum::"Entity Text Tone"::Formal, Format)); |
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.
InputPrompt := InputToken.AsValue().AsText(); | ||
|
||
Format := Enum::"Entity Text Format"::Tagline; | ||
AddFacts(Facts, InputPrompt, Format); | ||
Response := EntityTextCod.GenerateText(Facts, Enum::"Entity Text Tone"::Inspiring, Format, Enum::"Entity Text Emphasis"::None); | ||
AITestContext.SetTestOutput(PrepareOutput(BuildFactsText(Facts), Response, Enum::"Entity Text Tone"::Inspiring, Format)); |
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.
Here is the same code repeated again, and in the previous codeunit also. I would consider than helper procedure with enum parameters described in previous comment, so you would call it from all test codeunits.
NewLineChar: Char; | ||
begin | ||
NewLineChar := 10; | ||
Context := 'Here are some facts about the item:<br /><br />' + Facts.Replace(NewLineChar, EncodedNewlineTok); |
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.
Label?
Summary
AI Test for Marketing Text added.
Fixes # AB#521281