make the tests buildable and runnable with cabal and run said testsuites in CI #6
Conversation
facebook-github-bot
added
the
CLA Signed
alpmestan
force-pushed
the
alp/ci-test
branch
from
e741af7
to
1724246
Compare
| @@ -122,6 +123,29 @@ library | |||
| vector ^>=0.12.0.1 | |||
|
|
|||
| common test-common | |||
| hs-source-dirs: | |||
There was a problem hiding this comment.
Why are we compiling these modules for each test rather than using a library? The tests worked for me I think.
There was a problem hiding this comment.
The internal library thing was not working, at least with the cabal that we end up building with. I can get you the error a bit later, but this made me decide to stuff everything needed in the test-common section.
| common test-common | ||
| hs-source-dirs: test, test/github, test/gen-hs2, ../lib/test/gen-hs2, ../lib/test | ||
|
|
||
| other-modules: Facebook.Init |
There was a problem hiding this comment.
same question - why no library?
|
Following tips given in docker/docs#6075 -- I configured my local Docker service to be ipv6 capable and now all the lib/server tests pass! I'm down to just those 3 failing: The bad news is that Github Actions won't support ipv6 capable Docker environments anytime soon: actions/runner-images#668 And Travis' pricing changed recently, not positively. Is there a way we could perhaps fallback to ipv4 without too much hassle, in the tests? |
|
Re: the -- | We've patched the official aeson library to handle NaN. See #5183005
-- This test ensures that we notice if we forget to patch on upgrade.
parseNaN :: Test
parseNaN = TestCase $
case decode "[NaN]" of
Just [a :: Double] ->
assertBool "NaN" $ isNaN a
_ -> assertFailure "couldn't parse NaN"Aeson has this test input: The comment above presumably refers to the patch that you were talking about the other day. What do you want us to do here, omit this test for now? |
|
Regarding the regexTest :: Assertion
regexTest = assertBool "regex match test" $ regexMatchWord "lunedi"
-- ... in another module...
regexMatchWord :: Text -> Bool
regexMatchWord w = traceShow (ranges, w, Text.length w) $ ranges == [(0, Text.length w)]
where
regex = R.makeRegexOpts compOpts execOpts r
compOpts = PCRE.defaultCompOpt + PCRE.compCaseless
execOpts = PCRE.defaultExecOpt
r = "luned(\x00ec|i)" :: String
allMatches = R.match regex (Text.encodeUtf8 w) :: [[ByteString]]
(ranges, _, _) = foldl' g ([], w, 0) allMatches
g (ranges', s, offset) matches =
case map Text.decodeUtf8 matches of
[] -> (ranges', s, offset)
("":_) -> (ranges', s, offset)
(text:_group) ->
let (x,xs) = Text.breakOn text s
m = offset + Text.length x
n = Text.length text
s' = Text.drop n xs
newOffset = m + n
in (ranges' ++ [range], s', newOffset)I made this change to peek at what was going on: diff --git a/common/util/tests/EncodingLib.hs b/common/util/tests/EncodingLib.hs
index da25169..83e66da 100644
--- a/common/util/tests/EncodingLib.hs
+++ b/common/util/tests/EncodingLib.hs
@@ -12,8 +12,10 @@ import Data.List (foldl')
import Foreign.C hiding (peekCStringLen)
import Data.Text.Foreign
+import Debug.Trace
+
regexMatchWord :: Text -> Bool
-regexMatchWord w = ranges == [(0, Text.length w)]
+regexMatchWord w = traceShow (ranges, w, Text.length w) $ ranges == [(0, Text.length w)]The And the matching part of the input (all of it) does span from offset 0 ( However, I suspect this test is passing for you, so maybe my guess above is wrong, and somehow we should get |
We can |
A fix for this will be incoming shortly. |
I believe this test needs the locale to be set. But in any case, it's not actually testing anything in this library, so we should omit it - I'll fix this up and remove the test from the repository. |
alpmestan
force-pushed
the
alp/ci-test
branch
from
d9bdd18
to
544adbe
Compare
alpmestan
force-pushed
the
alp/ci-test
branch
from
544adbe
to
1ff9db6
Compare
There was a problem hiding this comment.
@simonmar has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
With the tip of this branch, all tests but (FWIW, looks like this problem appeared with hspec 2.5 -- see jcristovao/enclosed-exceptions#12) |
Summary: See #6 Github actions apparently has problems with IPv6, we weren't able to get these tests working using ::1 as the localhost address, so this diff makes the tests optionally use 127.0.0.1 instead. In fbcode we'll continue to use ::1. Reviewed By: anubhav94N Differential Revision: D25196206 fbshipit-source-id: 0b3af3b32a421a843bc2c2eb6fc988c0d8c842d4
|
I tried to re-export my version of your patch as a pull request, but it
seems that CI is failing at the checkout stage with
repository 'https://github.com/facebookincubator/hsthrift/' not found
Can you help me understand what's happening here?
https://github.com/facebookincubator/hsthrift/actions/runs/387416881
On Fri, 27 Nov 2020 at 14:28, Alp ***@***.***> wrote:
With the tip of this branch, all tests but rwvar pass for me, and
hopefully in CI too (ongoing, right now). I suspect the test driver we're
using here (hspec) and the one used on the FB side are different enough
in their approach to running tests (async or not, masking or not, etc).
Using hspec results in the main thread being deadlocked too and receiving
the exception from the RTS, whereas the one you use presumably protects
itself against that type of problem.
(FWIW, looks like this problem appeared with hspec 2.5 -- see
jcristovao/enclosed-exceptions#12
<jcristovao/enclosed-exceptions#12>)
We're not using hspec to run tests, we use HUnit directly I think. Perhaps
that might fix it?
Cheers
Simon
… —
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEU2RTDBEFUDAZKSWMFQ6TSR6ZQTANCNFSM4T7PA7NA>
.
|
|
Oh, I think it be a github issue:
actions/checkout#254
…On Fri, 27 Nov 2020 at 16:24, Simon Marlow ***@***.***> wrote:
I tried to re-export my version of your patch as a pull request, but it
seems that CI is failing at the checkout stage with
repository 'https://github.com/facebookincubator/hsthrift/' not found
Can you help me understand what's happening here?
https://github.com/facebookincubator/hsthrift/actions/runs/387416881
On Fri, 27 Nov 2020 at 14:28, Alp ***@***.***> wrote:
> With the tip of this branch, all tests but rwvar pass for me, and
> hopefully in CI too (ongoing, right now). I suspect the test driver we're
> using here (hspec) and the one used on the FB side are different enough
> in their approach to running tests (async or not, masking or not, etc).
> Using hspec results in the main thread being deadlocked too and
> receiving the exception from the RTS, whereas the one you use presumably
> protects itself against that type of problem.
>
> (FWIW, looks like this problem appeared with hspec 2.5 -- see
> jcristovao/enclosed-exceptions#12
> <jcristovao/enclosed-exceptions#12>)
>
We're not using hspec to run tests, we use HUnit directly I think. Perhaps
that might fix it?
Cheers
Simon
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#6 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAEU2RTDBEFUDAZKSWMFQ6TSR6ZQTANCNFSM4T7PA7NA>
> .
>
|
|
I believe this is all incorporated now. |
WIP -- the testsuites aren't green yet.
Current status:
unit-tests:rwvar:header-channel:dangling-pointer:server: