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

Refactor AdoTemplate rendering #3370

Merged
merged 5 commits into from
Aug 1, 2023

Conversation

tevoinea
Copy link
Member

@tevoinea tevoinea commented Aug 1, 2023

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

@codecov-commenter
Copy link

codecov-commenter commented Aug 1, 2023

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
24 checks passed
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
3 participants