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 on pypy2/3 plus irrelevant bug on tests discovered by the build #24

Merged
merged 2 commits into from Apr 23, 2015
Merged

Conversation

Digenis
Copy link
Contributor

@Digenis Digenis commented Apr 23, 2015

Tests succeed on pypy2/3 (thus added to travis)
except for some tests on sets
that were accidentally written as deterministic.
(I fixed them in the same PR to keep the build clean)


By the way,
while set(seq('abc')) gives the expected in the shell,
in tests I get an empty set and can only use .the set() method.
Probably assertSetEqual(result, set(expected)) doesn't work for the same reason.

Looks to me like a bug you may want to investigate.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.41% when pulling e6fd017 on Digenis:master into 43fe6a6 on EntilZha:master.

@EntilZha
Copy link
Owner

Good catch on the tests and thanks for adding PyPy.

My intuition is that what behavior you are seeing is related to how I did the initial implementation of generators from #17 on master. The problem at the moment is that an iterator is returned on transformations, so if that result is iterated on more than once, it returns nothing the second time. What I am unsure of is that the set result doesn't look like its being iterated over more than once... In any case this overall issue is being worked on in #20 and the lineage-rewrite branch.

PR looks good to go though.

EntilZha added a commit that referenced this pull request Apr 23, 2015
Test on pypy2/3 plus irrelevant bug on tests discovered by the build
@EntilZha EntilZha merged commit 24ad61f into EntilZha:master Apr 23, 2015
@EntilZha
Copy link
Owner

Would you mind making an issue with a code example of what is breaking? I am trying to reproduce with no luck.

@Digenis
Copy link
Contributor Author

Digenis commented Apr 23, 2015

Try this, it results in an empty set.
Dropped to pdb and retried without being able to reproduce it.

diff --git a/test/test_functional.py b/test/test_functional.py
index 78a0342..149dfe2 100644
--- a/test/test_functional.py
+++ b/test/test_functional.py
@@ -222,7 +222,7 @@ class TestChain(unittest.TestCase):
         result = seq([1, 1, 2, 3, 3]).union([1, 4, 5])
         expect = [1, 2, 3, 4, 5]
         self.assert_type(result)
-        self.assertSetEqual(result.set(), set(expect))
+        self.assertSetEqual(set(result), set(expect))
======================================================================
FAIL: test_union (test.test_functional.TestChain)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/digenis/src/ScalaFunctional/test/test_functional.py", line 225, in test_union
    self.assertSetEqual(set(result), set(expect))
AssertionError: Items in the second set but not the first:
1
2
3
4
5

Name               Stmts   Miss  Cover   Missing
------------------------------------------------
functional             8      0   100%   
functional.chain     307     12    96%   8-14, 121, 129, 870, 1004, 1012
------------------------------------------------
TOTAL                315     12    96%   
----------------------------------------------------------------------
Ran 79 tests in 0.712s

FAILED (failures=1)

@Digenis
Copy link
Contributor Author

Digenis commented Apr 23, 2015

Also, I noticed that the same pattern for dicts (not using assertDictEqual) so I pushed once more.
I can't reopen the PR, maybe you can try pulling like this to get the last commit:
git pull git@github.com:Digenis/ScalaFunctional.git master

@EntilZha
Copy link
Owner

I run the same thing without errors, might running on pypy vs standard python matter? I can either pull that commit or can merge another PR if you would like, either is fine with me.

@EntilZha
Copy link
Owner

Ya, I just installed pypy and it fails with that, but not with regular python. I can take a look and dive deeper later today/tonight.

@Digenis
Copy link
Contributor Author

Digenis commented Apr 23, 2015

No need for PR formalities for a follow up committed so soon :).
I tested on pypy3. I didn't try it in regular python. That surprises me.

@EntilZha
Copy link
Owner

Made an issue after some extensive debugging at #25

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

3 participants