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

feat(mvp-ado-extension-behavior): Add optional inputs to task.json and config #823

Merged
merged 5 commits into from
Sep 7, 2021

Conversation

jalkire
Copy link
Contributor

@jalkire jalkire commented Sep 3, 2021

Details

This PR adds the optional failOnAccessibilityError and repoServiceConnectionName inputs to our task.json and task-config files.

Motivation

We'll use these in future PRs for this feature.

Context

n/a

Pull request checklist

  • [n/a] Addresses an existing issue: Fixes #0000
  • Added relevant unit test for your changes. (yarn test)
  • Verified code coverage for the changes made. Check coverage report at: <rootDir>/test-results/unit/coverage
  • Ran precheckin (yarn precheckin)
  • (after PR created) The Accessibility Checks (pull_request) check should fail. All other checks should pass.

@jalkire jalkire requested a review from a team as a code owner September 3, 2021 21:59
@@ -55,6 +56,21 @@ describe(ADOTaskConfig, () => {
},
);

it.each`
inputOption | inputValue | expectedValue | getInputFunc
${'failOnAccessibilityError'} | ${true} | ${true} | ${() => taskConfig.getFailOnAccessibilityError()}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there is only one test case in this .each test. Do you see this test having more test cases in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could, I mostly wanted this to read similarly to the above test case because they are doing almost the same thing.

Copy link
Contributor

@lisli1 lisli1 left a comment

Choose a reason for hiding this comment

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

LGTM; one question inline

@jalkire jalkire merged commit 32206c5 into microsoft:main Sep 7, 2021
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.

2 participants