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.

Sign up for GitHub

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

Jump to bottom

Added processing of PHPUnit warnings as failures #48

Merged
merged 3 commits into from
Sep 14, 2021
Merged

Added processing of PHPUnit warnings as failures #48

merged 3 commits into from
Sep 14, 2021

Conversation

slavvka
Copy link

@slavvka slavvka commented Feb 6, 2020 •
edited
Loading

Added processing of PHPUnit warnings as failures since PHPUnit returns exit code 1 on warnings

Look at #47

@slavvka slavvka changed the title Added processing of PHPUnit warnings as failures since PHPUnit return… Added processing of PHPUnit warnings as failures Feb 6, 2020
@slavvka slavvka marked this pull request as ready for review February 6, 2020 23:12
@slavvka slavvka requested a review from baev February 6, 2020 23:14
@CLAassistant
Copy link

CLA assistant check
All committers have signed the CLA.

@slavvka
Copy link
Author

slavvka commented Apr 17, 2020

@baev Could you please review the PR?

@slavvka slavvka mentioned this pull request Apr 18, 2020
Merged
4 tasks
@eduard-sukharev
Copy link

Looks fine, but wouldn't it be better to have this as optional behavior?

@slavvka
Copy link
Author

slavvka commented Feb 5, 2021

Looks fine, but wouldn't it be better to have this as optional behavior?

Why? PHPUnit unconditionally returns exit code 1 on warnings. That means any CICD system using it will fail. With current behavior it'll fail without any records in an Allure report. Could you please give some use cases?

@ihor-sviziev
Copy link

@eduard-sukharev any updates on this?

@ihor-sviziev
Copy link

@baev, could you take a look at this PR? I think we should accept it.
To provide more context - Magento 2 using allure for generating reports and publishing the only allure reports. However, sometimes we're getting failing build, but in published reports, no info at all.

remorhaz
remorhaz approved these changes Apr 20, 2021
View reviewed changes
Copy link
Contributor

@remorhaz remorhaz left a comment

Choose a reason for hiding this comment

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

Well, Allure doesn't have "warning" test status anyway, and it's much better to process "warning" as "failure" than to ignore it.

@baev
Copy link
Member

baev commented Apr 21, 2021

looks good, @slavvka could you please rebase and I'll merge

@slavvka
Copy link
Author

slavvka commented May 19, 2021

@baev will do

@baev baev added the type:new feature New feature or request label Sep 14, 2021
@baev baev merged commit 5584e7e into allure-framework:master Sep 14, 2021
@baev baev mentioned this pull request Sep 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ihor-sviziev ihor-sviziev ihor-sviziev approved these changes

@baev baev baev approved these changes

@remorhaz remorhaz remorhaz approved these changes

Assignees
No one assigned
Labels
type:new feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@slavvka @CLAassistant @eduard-sukharev @ihor-sviziev @baev @remorhaz