Skip to content
This repository was archived by the owner on Nov 1, 2023. It is now read-only.

Refactor AdoTemplate rendering#3370

Merged
tevoinea merged 5 commits into
microsoft:mainfrom
tevoinea:tevoinea/FixTitleTruncation
Aug 1, 2023
Merged

Refactor AdoTemplate rendering#3370
tevoinea merged 5 commits into
microsoft:mainfrom
tevoinea:tevoinea/FixTitleTruncation

Conversation

@tevoinea

@tevoinea tevoinea commented Aug 1, 2023

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

What is this about?

Closes #3369
Closes #1692

Info on Pull Request

What does this include?

  • A new model RenderedAdoTemplate
  • Once per notification attempt, we render our AdoTemplate into a RenderedAdoTemplate. During that process, we can enforce limitations imposed by ADO such as a max title length
  • Later, when using the fields from the config, we don't need to remember/worry about re-rendering or making sure the rendered results meets the ADO limitations
  • Refactor AdoConnector to only work with a RenderedAdoTemplate to enforce the behavior from the previous 2 bullet points
  • AdoConnector no longer has the ability to render fields in a config
  • Split ExistingWorkItems into 2 parts: - Allowing us to test the query we generate
    1. Creating the ADO query
    2. Executing the query and returning results
  • A test to validate the ADO query we're generating
  • A test to validate that the title is truncated when rendering AdoTemplate to RenderedAdoTemplate

Validation Steps Performed

How does someone test & validate?

Validated in test instance:

  1. Start a new job
  2. Attach a notification config to the job and make the config so that it's guaranteed to truncate the title (I just repeated report.crash_site over and over)
  3. Create a debug notification for the job from step 1
  4. Verify a bug was created with the appropriate template and that the title was truncated
  5. Repeat step 3 with a new crash name
  6. Verify the bug from step 4 was updated with a comment containing the new crash name
  7. Verify a new bug was not created

Comment thread src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs
Comment thread src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs
@codecov-commenter

codecov-commenter commented Aug 1, 2023

Copy link
Copy Markdown

Codecov Report

Merging #3370 (5eb0b01) into main (f144544) will increase coverage by 0.17%.
Report is 5 commits behind head on main.
The diff coverage is 61.11%.

@@            Coverage Diff             @@
##             main    #3370      +/-   ##
==========================================
+ Coverage   31.69%   31.86%   +0.17%     
==========================================
  Files         308      308              
  Lines       37615    37659      +44     
==========================================
+ Hits        11921    12000      +79     
+ Misses      25694    25659      -35     
Files Changed Coverage Δ
...Service/ApiService/onefuzzlib/notifications/Ado.cs 27.16% <60.67%> (+17.82%) ⬆️
src/ApiService/ApiService/OneFuzzTypes/Model.cs 72.71% <100.00%> (+0.03%) ⬆️

... and 5 files with indirect coverage changes

@tevoinea tevoinea merged commit cfbcb16 into microsoft:main Aug 1, 2023
AdamL-Microsoft pushed a commit that referenced this pull request Aug 1, 2023
* .

* Add tests and complete refactor

* Remove done todo

* Log the query we created
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug when checking for existing work items with truncated title Enable debugging WIQL generated from ADO notification configs

3 participants