DistributedTaskScheduling / JobAdder

Source code of the JobAdder project
GNU General Public License v3.0
2 stars 1 forks source link

cli and tests for it #108

Closed FellowPlanter closed 4 years ago

FellowPlanter commented 4 years ago

New PR for the CLI (since I can't rebase on top of master in the other one) All tests passed locally and you can actually try out the CLI by calling print(UserClientCLIHandler(path to yaml file).get_command_from_cli().to_dict())

Now also includes user commands.

Fixes #71 Fixes #69 Fixes #68 Fixes #67

FellowPlanter commented 4 years ago

All tests pass locally, the travis checks throw the same error for all tests. Apparently the problem is the uid setter of the job, and the error is raised in the sql package for some reason.

Traceback (most recent call last): File "/home/travis/build/DistributedTaskScheduling/JobAdder/src/test/user/cli.py", line 38, in setUp job.uid = str(i) File "<string>", line 1, in __set__ File "/home/travis/build/DistributedTaskScheduling/JobAdder/src/ja/common/job.py", line 154, in uid self._uid = value File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py", line 268, in __set__ instance_state(instance), instance_dict(instance), value, None AttributeError: 'Job' object has no attribute '_sa_instance_state'

JohannesGaessler commented 4 years ago

It seems to me like the unittests for the mock database are making some sort of permanent change to the Job class. This is the responsibility of @nikolatzotchev to debug.

JohannesGaessler commented 4 years ago

Disregard my last post, I found out what the problem is. The problem is that commands and jobs are being set as static properties of CLITest. As such they get initialized on import, before the tests for the mock database are being run. The mock database tests then apparently do something weird to the static properties. If you just initialize commands and jobs as an attribute of the object in setUp (something like self.commands = ...) it works. (The paths for p.yaml and add.yaml is broken when running the tests from the main test directory though.)

ammen99 commented 4 years ago

One general issue that I noticed when compiling statistics about our project: you are missing unit tests for the Cancel command (also Query seems to be missing some lines, but maybe we can test them in the QA). Take a look here: https://travis-ci.org/DistributedTaskScheduling/JobAdder/jobs/657678748#L730

JohannesGaessler commented 4 years ago

The branch you force-pushed is based off of an old commit of the master branch. Did you perhaps forget to pull from the master branch before rebasing?

JohannesGaessler commented 4 years ago

After a squash and a rebase I think this is good to merge.

ammen99 commented 4 years ago

@M1keReck You should also rebase on top of the master branch to fix most of the errors, and I also think you have one line which is too long, after that we can merge.

FellowPlanter commented 4 years ago

I actually wanted to add those tests but I am struggling with the MockDatabase initialization, if I can't solve it tonight, I'll bring this up in the meeting.