Resolves #66

[RickFqt]

• Resolves #66.

[alvarofpp]

One last thing I forgot to mention: all the code must be covered by unit tests.

[RickFqt]

I noticed that the commits are not in the right pattern. There is no capital letter after the :.

To fix this, should I undo the commits, or rename them with git rebase?

[alvarofpp]

To fix this, should I undo the commits, or rename them with git rebase?

I recommend using git rebase to rewrite the commits, then git push -f on your branch.

[alvarofpp]

Just a reminder of something you should do at the end of all this: use git rebase to put everything into a single commit.

[RickFqt]

One last thing I forgot to mention: all the code must be covered by unit tests.

About these unit tests, is there any reference that can be used as a model to do them? Also, will they work like this lint job that runs to check if the code is okay to merge?

Another question: how should we make tests for abstract methods in these base classes we are going to create? I understand how they should work on implementations of these classes (like a Starcraft II environment class, for example), but I don't see how to do them in the base classes.

[alvarofpp]

@RickFqt

About these unit tests, is there any reference that can be used as a model to do them?

Another question: how should we make tests for abstract methods in these base classes we are going to create? I understand how they should work on implementations of these classes (like a Starcraft II environment class, for example), but I don't see how to do them in the base classes.

This is our first abstract class, so we don't have any tests to use as a base. I found some links on the internet that might be useful for you:

• https://clamytoe.github.io/articles/2020/Mar/12/testing-abcs-with-abstract-methods-with-pytest/
• https://copyprogramming.com/howto/python-testing-an-abstract-base-class

Also, will they work like this lint job that runs to check if the code is okay to merge?

Yes, we'll have a job to check test coverage. You can run all of this (linter and tests) locally using the task lint, task unit-test, and task test-coverage commands.

[RickFqt]

Yes, we'll have a job to check test coverage.

You can run all of this (linter and tests) locally using the task lint, task unit-test, and task test-coverage commands.

Tried to run linter locally here, but keep getting this error message: I searched a little bit about this error message, but couldn't find anything to solve it. Do you know what might be causing it?

Also got error messages for running the test commands, but I think that is because we don't have any tests implemented yet. Is that right?

[alvarofpp]

I searched a little bit about this error message, but couldn't find anything to solve it. Do you know what might be causing it?

We can ignore this error for now. To do this, add the line below to the beginning of the failed files:

# yamllint disable rule:new-lines

And add the line below at the end of the failed files:

# yamllint enable

Also got error messages for running the test commands, but I think that is because we don't have any tests implemented yet. Is that right?

That's exactly why.

[RickFqt]

That's exactly why.

So, about the unit tests, where should they be implemented? I'm trying to do that inside tests/units, but keep getting the same error as before.

[alvarofpp]

So, about the unit tests, where should they be implemented? I'm trying to do that inside tests/units, but keep getting the same error as before.

You should create a Fake class to inherit from EnvironmentBase and apply the tests to it. It's OK to create this class inside the test file, as it will only be used for this purpose.

[alvarofpp]

Another thing: the name abenv.py is not explicit. I recommend renaming the file environment_base.py.

[RickFqt]

You should create a Fake class to inherit from EnvironmentBase and apply the tests to it. It's OK to create this class inside the test file, as it will only be used for this purpose.

Based on the links recommended, I wrote the following test:

from abc import ABCMeta
from urnai.environments.environment_base import EnvironmentBase
import unittest

class TestEnvironmentBase(unittest.TestCase):

    def test_environment_base(self):
        EnvironmentBase.__abstractmethods__ = set()

        class EnvironmentDummy(EnvironmentBase):
            def __init__(self, id: str, render=False, reset_done=True):
                super().__init__(id, render, reset_done)

        d = EnvironmentDummy("id")
        start_return = d.start()
        step_return = d.step("action")
        reset_return = d.reset()
        close_return = d.close()
        assert isinstance(EnvironmentBase, ABCMeta)
        assert start_return is None
        assert step_return is None
        assert reset_return is None
        assert close_return is None

I wasn't sure what to put into the "id" and "action" paramaters from the initializer and step, so I just put these strings. ID is supposed to be a string, but I'm not sure about the action parameter.

Also, I wasn't able to use task unit-test to run this test, although I could run it normally with pytest locally here. Is there a reason for that?

[alvarofpp]

You're going in the right direction, a few suggestions:

• I think the function name could be more explicit than just repeating the class name.
• It would avoid the use of Dummy, the reason for which you can see here.

Also, I wasn't able to use task unit-test to run this test, although I could run it normally with pytest locally here. Is there a reason for that?

Can you post a printout of what happens when you run task unit-test?

💡 Tip: you can upload the unit test code so we can see it better. Before the merge, you can merge commits, change their texts, etc.

[RickFqt]

Can you post a printout of what happens when you run task unit-test?

The error is just like in here:

Also got error messages for running the test commands, but I think that is because we don't have any tests implemented yet. Is that right?

💡 Tip: you can upload the unit test code so we can see it better. Before the merge, you can merge commits, change their texts, etc.

I've just tried to upload the test code, but keep getting this error:

[alvarofpp]

This error occurs because of githook. Try running the push command, but add the --no-verify flag.

[alvarofpp]

@RickFqt I've taken the liberty of making the changes directly in the code (I've taken the opportunity to update our linter image, https://github.com/UFRN-URNAI/docker-images/pull/6 ).

[RickFqt]

@RickFqt I've taken the liberty of making the changes directly in the code (I've taken the opportunity to update our linter image, UFRN-URNAI/docker-images#6 ).

Now, task test-coverage works, but it shows that 0% of the code is covered:

Also, when I try task unit-test, I now receive another error: TaskUnitTestError.txt

One more thing: I tried to use task-lint just to check if it was working, and I'm not sure if it is detecting problems in the code correctly. Before, it gave me a check mark that everything was correct, but now it doesn't seem to show anything. For example, I tried to delete some comments, add some random newlines in environment_base.py and ran task-lint, but it didn't warn anything. It was just like this:

[alvarofpp]

I tried to reproduce that test error locally and was unsuccessful. Try deleting the .coverage file and running task unit-test test-coverage again. Also try building the image again.

The linter is indeed strange, I'll check it at some point.

[alvarofpp]

@RickFqt I fixed the problem with the Python linter, I had forgotten to enter which rules it should consider when evaluating the code. The Docker image has already been updated and I've fixed the files here in your branch.

[RickFqt]

@RickFqt Try running the tests for your code locally. If all goes well, you can merge it.

Locally, it went well. Thanks!