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

Test custom getopt only when it is enabled. #2259

Closed
wants to merge 1 commit into from

Conversation

remysucre
Copy link

The custom getopt is enabled only when SOUFFLE_CUSTOM_GETOPTLONG is set; that flag defaults to OFF, so the test for getopt would fail. That test should only run when SOUFFLE_CUSTOM_GETOPTLONG is ON.

The custom `getopt` is enabled only when `SOUFFLE_CUSTOM_GETOPTLONG` is set; that flag defaults to OFF, so the test for `getopt` would fail. That test should only run when `SOUFFLE_CUSTOM_GETOPTLONG` is ON.
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #2259 (ac0cf71) into master (418a194) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2259      +/-   ##
==========================================
- Coverage   75.15%   75.12%   -0.04%     
==========================================
  Files         455      455              
  Lines       28465    28465              
==========================================
- Hits        21392    21383       -9     
- Misses       7073     7082       +9     
Impacted Files Coverage Δ
src/include/souffle/profile/ProfileEvent.h 92.85% <0.00%> (-5.11%) ⬇️
src/include/souffle/utility/ParallelUtil.h 75.19% <0.00%> (-1.56%) ⬇️
src/include/souffle/datastructure/BTree.h 96.63% <0.00%> (-0.43%) ⬇️

@quentin
Copy link
Member

quentin commented Apr 19, 2022

Hi,
The test is designed to work even when Souffle is not built with the custom implementation.
If the test does not work properly, please let me know and I'll try to fix.

@remysucre
Copy link
Author

I'm attaching the test log below. I ran the tests on an ubuntu:20.04 docker on Apple M1.
LastTest.log

@quentin
Copy link
Member

quentin commented Apr 19, 2022

The messages suggest there might be an issue with the comparison of char with -1. It must be related to APPLE ARM architecture. It is as if char is unsigned, that would explain why the error message displays \FF instead of -1.

GetOptLong
		TEST FAILED @ line 487 : expected c == -1 where
			c evaluates to \FF
			-1 evaluates to -1

@remysucre
Copy link
Author

I see. There are other issues on ARM related to signed/unsigned integers, so I'm going to switch to x86 for now. Will move this to an issue.

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.

None yet

2 participants