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

ReQL test runner using existing server is broken #2795

Closed
danielmewes opened this issue Aug 1, 2014 · 4 comments
Closed

ReQL test runner using existing server is broken #2795

danielmewes opened this issue Aug 1, 2014 · 4 comments
Assignees
Milestone

Comments

@danielmewes
Copy link
Member

test/rql_test/test_runner supports the -c and -d options to connect to an already running server. That is very useful to run tests against a server running under Valgrind for example.

However this mode of operation seems to be broken.

I'm getting this error:

Traceback (most recent call last):
  File "./test-runner", line 761, in <module>
    main()
  File "./test-runner", line 540, in main
    build_directory = options.executable_path
AttributeError: Values instance has no attribute 'executable_path'

The executable_path option doesn't seem to exist anymore though. If I change the code to specify the path manually the next problem is

Traceback (most recent call last):
  File "./test-runner", line 761, in <module>
    main()
  File "./test-runner", line 541, in main
    servers = RethinkDBRunningServer(options.driver_port, options.cluster_port)
  File "./test-runner", line 367, in __init__
    self.rethinkdb_executable_ = rethinkdb_executable
NameError: global name 'rethinkdb_executable' is not defined

That line just seems to be unnecessary, since self.rethinkdb_executable_ isn't used anywhere as far as I can see.

Ping @larkost

@danielmewes danielmewes added this to the tests milestone Aug 1, 2014
@danielmewes
Copy link
Member Author

Another problem:

RethinkDB server failed: RethinkDBRunningServer instance has no attribute 'stop'== Failed polyglot/match.py test with result code 1 (/home/ssd3/daniel/rethinkdb/test/rql_test/src/match.yaml) Output

@Tryneus
Copy link
Member

Tryneus commented Aug 1, 2014

I ran into the same problem and noticed those were unused, so I got this working with the following diff:

--- test/rql_test/test-runner
+++ test/rql_test/test-runner
@@ -357,21 +357,20 @@
                 else:
                     expected = expected['cd']
             return Langs.repr_expected_for(lang, expected)

         return None

 class RethinkDBRunningServer:
     def __init__(self, driver_port, cluster_port):
         self.driver_port_ = driver_port
         self.cluster_port_ = cluster_port
-        self.rethinkdb_executable_ = rethinkdb_executable

     def driver_port(self):
         return self.driver_port_

     def cluster_port(self):
         return self.cluster_port_

     def executable(self):
         return 'NOT_AVALIBLE_FOR_RUNNING_SERVERS'

@@ -530,21 +529,20 @@

     options, args = parser.parse_args()

     # - options validation

     # cluster_port/driver_port

     if (options.cluster_port is not None or options.driver_port is not None) and None in (options.cluster_port, options.driver_port):
         parser.error('If either of the following options are used, both must be specifed: -c/--cluster-port, -d/--driver-port')
     if options.driver_port is not None:
-        build_directory = options.executable_path
         servers = RethinkDBRunningServer(options.driver_port, options.cluster_port)
         # TODO: validate that this is actually a running server

         if options.shards != 1:
             parser.error('The -c/--cluster-port and -d/--driver-port options can not be used with -s/--shards')

     # - add filters

     for newFilter in args:
         try:

Didn't hit that problem with stop, though.

@larkost
Copy link
Collaborator

larkost commented Aug 1, 2014

Some of this is already in the Python3 version that I am in the middle of merging right now. Once the validation runs I am doing right now get through (and the merge is completed) I will look at this.

@larkost
Copy link
Collaborator

larkost commented Aug 11, 2014

All of the changes for this are already in next from other checkins.

@larkost larkost closed this as completed Aug 11, 2014
@larkost larkost modified the milestones: tests, 1.14 Aug 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants