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

Allow tests to run on systems with case-insensitive filesystems and newer compilers. #208

Merged
merged 4 commits into from
Sep 8, 2023

Conversation

jboynes
Copy link
Contributor

@jboynes jboynes commented Sep 7, 2023

This change allows the test suite to run on desktop machines with case-insenstive filesystems (e.g. macOS and Windows) without needing a Linux VM or special mount point.

The main issue is the conflict between "String.h" in the api directory and the standard "string.h" include; the API's one gets matched when included with #include <String.h>" which does not happen in a core because the include path points to the parent of the api directory. So the main change here is to alter the include path for the tests to the parent of the api directory to match. Unfortunately this meant modifying all the tests to include api/ in the header filename.

Testing this on macOS with XCode 13.3 (native cc which is clang 14.x) and GCC 13 also tripped over stricter warnings related to the deprecation of sprintf and the self-move test for String. The PR includes #pragma statements to suppress those warnings as I didn't want to change production code in this PR, saving it for separate patch.

I confirmed the tests still pass on Ubuntu with GCC 12.

@codecov-commenter
Copy link

codecov-commenter commented Sep 7, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (f77c4b5) 95.52% compared to head (da415e6) 95.52%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #208   +/-   ##
=======================================
  Coverage   95.52%   95.52%           
=======================================
  Files          16       16           
  Lines        1072     1072           
=======================================
  Hits         1024     1024           
  Misses         48       48           
Files Changed Coverage Δ
api/IPAddress.cpp 94.52% <ø> (ø)
api/deprecated-avr-comp/avr/dtostrf.c.impl 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.

Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

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

Would merge, pending adjustments of include-path.

test/include/MillisFake.h Outdated Show resolved Hide resolved
Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@aentinger aentinger merged commit 66aa7db into arduino:master Sep 8, 2023
3 checks passed
@jboynes jboynes deleted the native_tests branch September 8, 2023 07:23
@per1234 per1234 added the bug label Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants