[
https://issues.apache.org/jira/browse/CASSANDRA-14975?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16744646#comment-16744646
]
Joseph Lynch commented on CASSANDRA-14975:
------------------------------------------
First pass review.
Overall:
* It looks like the cassandra branch is testing against your cassandra-dtest
[cqlsh_tests|https://github.com/apache/cassandra/compare/trunk...aweisberg:14975-trunk?expand=1#diff-1d37e48f9ceff6d8030570cd36286a61R307]
branch but you linked to your cassandra-dtest
[14975|https://github.com/apache/cassandra-dtest/compare/master...aweisberg:14975?expand=1]
branch. Did you mean to do that?
Dtest Code (14975 branch):
* From what I can tell {{assertCsvResultEqual}} is never called with
{{ignore_order}} False? Can we just remove that option and just always ignore
the order?
* I'm slightly confused why the ordering of some of these return values has
changed, for example
[here|https://github.com/apache/cassandra-dtest/compare/master...aweisberg:14975?expand=1#diff-2d1d32d5724e8f118d09beb660163899R1862]?
Did the COPY operator start returning csv rows in non lexicographic order or
some such?
* Looks like you need to tag Ryan McGuire [to
review|https://github.com/apache/cassandra-dtest/compare/master...aweisberg:14975?expand=1#diff-89bf909e4521aedef94efb3d8fd129f4R309]?
* Did you mean to add a dependency on [cql|https://pypi.org/project/cql/] in
[requirements.txt|https://github.com/apache/cassandra-dtest/compare/master...aweisberg:14975?expand=1#diff-b4ef698db8ca845e5845c4618278f29aR4]?
From what I can tell it isn't used in cassandra-dtest (it it used in
cassandra's pylib or something)?
* Nitpick: Probably shouldn't supply the encoding parameter
[here|https://github.com/apache/cassandra-dtest/compare/master...aweisberg:14975?expand=1#diff-89bf909e4521aedef94efb3d8fd129f4R192]
since we don't supply it elsewhere in the test function fwict
* Nitpick: you should probably not use '\' and instead use implicit
continuations e.g.
[here|https://github.com/apache/cassandra-dtest/compare/master...aweisberg:14975?expand=1#diff-2d1d32d5724e8f118d09beb660163899R3083].
You can just do:
{noformat}
assert (
len(records) == res
), "Failed to import one or more rows, expected {} but got
{}".format(len(records), res)
{noformat}
Cassandra code (14975 branch):
* It looks like we dep six, but from what I can tell we don't ever use it. In
that case we could drop importing it in {{formatting.py}}. I'm assuming that we
don't actually have six which is why we re-implement e.g.
[six.StringIO|https://pythonhosted.org/six/#six.StringIO]
[here|https://github.com/apache/cassandra/compare/trunk...aweisberg:14975-trunk?expand=1#diff-274d87311ad5258d95b795115b18807cR30]
* You probably want a {{from \_\_future\_\_ import print_function}} in
wcwidth.py and util.py, I think those print statements are working because of
implied continuations ...
I'm still working on understanding the pylib formatter code, should have better
feedback on the cassandra branch tomorrow once I understand it more.
> cqlsh dtests are not running in CircleCI
> ----------------------------------------
>
> Key: CASSANDRA-14975
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14975
> Project: Cassandra
> Issue Type: Test
> Components: Test/dtest
> Reporter: Ariel Weisberg
> Assignee: Ariel Weisberg
> Priority: Major
> Fix For: 3.0.x, 3.11.x, 4.0.x
>
>
> Right now the dtests skip these tests because most don't pass. Originally
> they weren't being collected because the test files end in_tests.py instead
> of _test.py, but I fixed that by adding _tests.py to the list of recognized
> patterns.
> Get them all running in CircleCI. Nothing special they will be part of the
> existing dtest jobs.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
