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

[MavenV2, MavenV3]: Fixed bug when task succeeds on tests failure #14658

Merged
merged 13 commits into from
Apr 1, 2021

Conversation

matskan
Copy link
Contributor

@matskan matskan commented Mar 25, 2021

Task name: MavenV2, MavenV3

Description: Maven task succeeded when tests failed. It was caused by incorrect exception handling on last stage of build.
Now the build will fail if tests don't finish successfully.
Added new optional input: allowBrokenSymlinks. If it is set false the task will fail when it finds bad symbolic link during publishing tests. For backward-combability it is set true by default to ignore incorrect symbolic links when task tries to publish tests.

Documentation changes required: Yes
Added unit tests: No
Attached related issue: #14514

Checklist:

  • Task version was bumped - please check instruction how to do it
  • Checked that applied changes work as expected
  • There’s no risky dependency updates
  • Changes has been tested
  • Enough test coverage for changes, and current test coverage for task doesn't look poor
  • We understand how tasks is working, how changes affect task behavior
  • There is no breaking changes
  • There is no any other concerns
  • I have not discovered any new uncovered test/use cases

✔️ Maven V3 succeeds when tests are passed and there is a bad symlink when task tries to publish tests (allowBrokenSymlinks = true)
Maven V3 fails when tests are passed (allowBrokenSymlinks = false). There is a bad symlink when task tries to publish tests
Maven V3 fails when tests are failed

✔️ Maven V2 succeeds when tests are passed and there is a bad symlink when task tries to publish tests (allowBrokenSymlinks = true)
Maven V2 fails when tests are passed (allowBrokenSymlinks = false). There is a bad symlink when task tries to publish tests)
Maven V2 fails when tests are failed

✔️ On canary app

@matskan matskan self-assigned this Mar 25, 2021
@matskan matskan requested a review from leantk as a code owner March 25, 2021 13:00
@ghost
Copy link

ghost commented Mar 25, 2021

CLA assistant check
All CLA requirements met.

@matskan matskan marked this pull request as draft March 25, 2021 13:44
@matskan matskan marked this pull request as ready for review March 25, 2021 14:01
@alexander-smolyakov alexander-smolyakov requested a review from a team March 25, 2021 14:02
@matskan matskan changed the title #14514: Maven task succeeds on maven build failure [MavenV2, MavenV3]: Fixed bug when task succeeds on maven build failure Mar 25, 2021
@matskan matskan changed the title [MavenV2, MavenV3]: Fixed bug when task succeeds on maven build failure [MavenV2, MavenV3]: Fixed bug when task succeeds on tests failure Mar 25, 2021
@vladislav-ryzhov
Copy link
Contributor

Manually tested these changes in this pipeline https://dev.azure.com/v-vryzhov/test/_build?definitionId=2 (#20210326.7, #20210326.6).
Tested MavenV2 and MavenV3 version with a basic java project. Build worked as expected: tests fail and the task fails too.

@anatolybolshakov anatolybolshakov requested a review from a team March 29, 2021 11:13
Copy link
Contributor

@anatolybolshakov anatolybolshakov left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@anatolybolshakov anatolybolshakov requested a review from a team March 29, 2021 15:50
Copy link
Contributor

@DaniilShmelev DaniilShmelev left a comment

Choose a reason for hiding this comment

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

Tested these changes manually, everything is working as expected:
Maven V2 test failures before
Maven V3 test failures before
Maven V2 test failures after
Maven V3 test failures after
Don't allow broken symlinks
Allow broken symlinks

Take a look at the suggestion, but other than that LGTM!

Tasks/MavenV2/task.json Outdated Show resolved Hide resolved
@matskan matskan merged commit 354883e into master Apr 1, 2021
@matskan matskan linked an issue Apr 2, 2021 that may be closed by this pull request
@starkmsu starkmsu deleted the maven-14514 branch July 14, 2023 12:00
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.

4 participants