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

Keep track of ignored errors, resolves #2533 #2681

Merged
merged 4 commits into from
Jun 7, 2022
Merged

Conversation

NeoTheThird
Copy link
Member

Users are often reluctant to report the first error they run into and chose to ignore it. If something does not work, they report it later and the error gets buried in the log. We should keep track of these and include them in the bug reports.

If any errors have been ignored, the PASS reporting option should be disabled. Even if everything worked, the correct status to report in this case is WONKY, as a workaround was required.

@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Merging #2681 (352f894) into master (60d2710) will increase coverage by 0.66%.
The diff coverage is 100.00%.

❗ Current head 352f894 differs from pull request most recent head 95d1041. Consider uploading reports for the commit 95d1041 to get more accurate results

@@            Coverage Diff             @@
##           master    #2681      +/-   ##
==========================================
+ Coverage   76.61%   77.27%   +0.66%     
==========================================
  Files          29       29              
  Lines         962      968       +6     
==========================================
+ Hits          737      748      +11     
+ Misses        225      220       -5     
Impacted Files Coverage Δ
src/core/core.js 72.05% <100.00%> (-0.61%) ⬇️
src/lib/errors.js 75.00% <100.00%> (+35.00%) ⬆️
src/lib/reporter.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60d2710...95d1041. Read the comment docs.

@NeoTheThird NeoTheThird marked this pull request as ready for review June 7, 2022 11:54
maciek134
maciek134 previously approved these changes Jun 7, 2022
Copy link
Contributor

@maciek134 maciek134 left a comment

Choose a reason for hiding this comment

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

Turns out I only have two pretty opinionated comments, LGTM otherwise so feel free to ignore :)

src/lib/__mocks__/mainEvent.js Outdated Show resolved Hide resolved
src/lib/errors.js Outdated Show resolved Hide resolved
src/lib/errors.js Outdated Show resolved Hide resolved
src/lib/errors.js Outdated Show resolved Hide resolved
@NeoTheThird NeoTheThird merged commit 5f5a494 into master Jun 7, 2022
@NeoTheThird NeoTheThird deleted the errors-store branch June 7, 2022 18:05
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