Skip to content
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

update XmlPeek and XmlPoke tasks #9194

Merged
merged 7 commits into from
Oct 4, 2023

Conversation

jrdodds
Copy link
Contributor

@jrdodds jrdodds commented Aug 31, 2023

Fixes #9140

Context

Add [Required] to parameters.

Changes Made

  • XmlPeek Task
    • Change Query parameter.
    • Remove redundant Dispose that was flagged by the analyzer.
    • Change XmlPeek.XmlInput class from Internal to private sealed and change access of some members
    • Minor cleanup changes
  • XmlPoke Task
    • Change Query parameter.
    • Change XmlInputPath parameter.
    • Minor cleanup changes
  • XmlPeek_Tests class
    • Add new PeekWithNoParameters test
  • XmlPoke_Tests class
    • Remove PokeMissingParams test
      • The test was defined as a [Fact] and used a for loop to test 4 distinct cases
      • The test expected ArgumentNullException to be thrown
    • Add 4 new tests, one for each of the four cases:
      • PokeWithNoParameters
      • PokeWithMissingRequiredQuery
      • PokeWithMissingRequiredXmlInputPath
      • PokeWithRequiredParameters (completes the replacement of PokeMissingParams but might be a redundant test)

Testing

Tested on macOS 12 and Windows 11.
Tested with the test project files shown in issue 9140.
Created and ran unit tests

Notes

@jrdodds jrdodds marked this pull request as ready for review August 31, 2023 12:21
@rainersigwald rainersigwald added this to the VS 17.9 milestone Sep 19, 2023
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome, thanks!

src/Tasks/XmlPeek.cs Show resolved Hide resolved
@jrdodds
Copy link
Contributor Author

jrdodds commented Oct 2, 2023

Created #9298 for an unreliable unit test that broke the Windows Core build.

If someone with access can re-run the Windows Core build, I expect it will pass.

@rainersigwald rainersigwald merged commit 1aba44b into dotnet:main Oct 4, 2023
8 checks passed
@jrdodds jrdodds deleted the XmlPeekPokeRequiredParams branch October 5, 2023 14:38
bulatgrzegorz pushed a commit to bulatgrzegorz/selective-condition-evaluator that referenced this pull request Oct 16, 2023
Add `[Required]` to parameters.

- `XmlPeek` Task
  - Change `Query` parameter.
  - Remove redundant `Dispose` that was flagged by the analyzer.
  - Change XmlPeek.XmlInput class from `Internal` to `private sealed` and change access of some members
  - Minor cleanup changes
- `XmlPoke` Task
  - Change `Query` parameter.
  - Change `XmlInputPath` parameter.
  - Minor cleanup changes
- XmlPeek_Tests class
  - Add new `PeekWithNoParameters` test
- XmlPoke_Tests class
  - Remove `PokeMissingParams` test
    - The test was defined as a `[Fact]` and used a for loop to test 4 distinct cases
    - The test expected `ArgumentNullException` to be thrown
  - Add 4 new tests, one for each of the four cases:
    - `PokeWithNoParameters`
    - `PokeWithMissingRequiredQuery`
    - `PokeWithMissingRequiredXmlInputPath`
    - `PokeWithRequiredParameters` (completes the replacement of `PokeMissingParams` but might be a redundant test)

Fixes dotnet#9140.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: XmlPeek and XmlPoke tasks when optional parameter is not provided, throw unhandled exceptions
3 participants