-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add classes to uni/lib that implement fixed point arithmetic. #363
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
################################################################################# | ||
# | ||
# This file is released under the terms of the GNU LIBRARY GENERAL PUBLIC LICENSE | ||
# (LGPL) version 2. The licence may be found in the root directory of the Unicon | ||
# source directory in the file COPYING. | ||
# | ||
# Fixed Point Arithmetic class tests (without operator overloading) | ||
# -------------------------------------------------------------------------------- | ||
|
||
procedure main(args) | ||
if not(&features == "large integers") then | ||
stop("large integers not supported") | ||
|
||
runTests(args) | ||
end | ||
|
||
# Turn off operator overloading | ||
$undef _OVLD | ||
$include "FxPtTests.icn" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
################################################################################# | ||
# | ||
# This file is released under the terms of the GNU LIBRARY GENERAL PUBLIC LICENSE | ||
# (LGPL) version 2. The licence may be found in the root directory of the Unicon | ||
# source directory in the file COPYING. | ||
# | ||
# Fixed Point Arithmetic class tests (with operator overloading) | ||
# -------------------------------------------------------------------------------- | ||
|
||
procedure main(args) | ||
if not(&features == "large integers") then | ||
stop("large integers not supported") | ||
|
||
if not (&features == "operator overloading") then | ||
stop("operator overloading not supported") | ||
|
||
runTests(args) | ||
end | ||
|
||
$include "FxPtTests.icn" | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
|
||
include ../Makedefs | ||
|
||
ifneq ("$(wildcard $(UC))","") | ||
TARGETS=$(patsubst %.icn,%,$(wildcard *.icn)) | ||
|
||
SKIP= tester FxPtTests\ | ||
audio dbi list_test nlm q rec test testnlm clin_err hello mintest ovld q2 sel to | ||
else | ||
skip: | ||
@echo "Unicon compiler not found" | ||
|
||
TARGETS=skip | ||
endif | ||
|
||
Test: DoTest | ||
|
||
include ../Makefile.test | ||
|
||
FxPtTest: FxPtTest.icn tester.u | ||
|
||
FxPtTest_OVLD: FxPtTest_OVLD.icn tester.u | ||
|
||
tester.u: tester.icn | ||
$(UC) -s -u -c $^ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test is failing because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But since it is also wrong for every other test in the test suite -- I guessed that the problem is in the Debian-Pkg configuration, not this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that is a separate issue, but as you can see, the tests above ignore. And only your PR fails the tests. Only the new Makefile attempts to compile the test in the subfolder. |
||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
-------------------------------------------------- | ||
Fixed Point Tests Start | ||
Fixed Point Tests Passed = 454 Failed = 0 | ||
-------------------------------------------------- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
-------------------------------------------------- | ||
Fixed Point Tests Start | ||
Fixed Point Tests Passed = 586 Failed = 0 | ||
-------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us not mask this error. I opened a PR to fix the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, the debian package test , builds a debian package and installs it. The unicon used should be the installed version, and that is determined by setting the UC variable in the test environment. That was correct in once place, but it looks like another instance was added at some point without taking the environment variable into account.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm genuinely at a loss here. I said "The problem is in the Debian-Pkg". You said "but your test fails and the others don't" (and didn't merge the PR). So I fixed the test to not fail. Then you said "let's not mask the error" and fixed the Debian-Pkg problem (and still didn't merge this PR). I could guess what modifications are needed (if any) in order to merge this PR, or you could say explicitly: I very much prefer the latter. Just to be clear, after PR367 is merged, either this version of PR363 or the previous version will both work -- which one do you want? If something else, then please say what the something else is.
As far as I can see, the other tests also use a variant of the technique I used to prevent the failure. If there's something wrong with it then there's also something wrong with the other tests (although they are testing for a feature, the skip is also selected if Unicon is absent).
I've merged PR367, rebased this commit on top of PR367 and force pushed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the confusion. It was obvious there is an issue in the debian pkg platform, not funding the unicon binaries. But all other tests were passing. There is no explicit "skip if we don't have Unicon", so I figured we handle that case globally and that there was still a separate issue in the PR. When you added the explicit step and it worked, I decided to go fix the platform issue.
To be clear, we don't want to have explicit skips if unicon doesn't exist, failing in that case is fine. If we get to the point where we don't have unicon binaries then we have a more fundamental issue in the test.