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

Fix false positive result where test passed even though gcc failed with error #342

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hlovdal
Copy link
Contributor

@hlovdal hlovdal commented Dec 29, 2022

Even if gcc failed with errors for some of the files compiled for unit test, the test would still be reported as successful. This is fixed by properly increasing @failure_count.

Also an additional fix for the result calculation for all_arduino_library_dependencies!. It is a long time since I made that commit so I do not exactly remember the details but reading it now accumulating with [n] inside the map section looks more correct.


Updates to CHANGELOG.md will trigger conflicts for this branch (failure_handling.m, rebased on top of master) when #338 is completed (or if this is done first, then #338 will get conflicts).

There is an alternative branch failure_handling applied in series with other branches that does not have those conflicts when #338 is completed. Let me know if you rather want that one merged (later).

@ianfixes
Copy link
Collaborator

Can you point me to an example of where this is happening and/or how I can reproduce it locally?

Also, I'm not clear on how the change to cpp_library.rb relates to the fix here.

end.flatten
(additional_libraries + recursive).uniq
recursive.uniq
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you have a test case or some performance metric that relates to this change? Just trying to figure out whether this is an improved implementation or a bugfix

@ianfixes
Copy link
Collaborator

I cherry-picked 5df7166 into #338 but I'm reluctant to change the "additional libraries" recursive function without a unit test in place.

@ianfixes ianfixes marked this pull request as draft December 31, 2022 19:50
@ianfixes ianfixes added the question Further information is requested label Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants