allenai / ScienceWorld

ScienceWorld is a text-based virtual environment centered around accomplishing tasks from the standardized elementary science curriculum.
https://sciworld.apps.allenai.org/
Apache License 2.0
199 stars 24 forks source link

Adding flake8, pytest, tox, and github actions #59

Closed AndKaminer closed 8 months ago

AndKaminer commented 8 months ago

As the title says.

Never worked with any of these technologies, so this is an amalgamation of various tutorials. It works fine on my branch, but I wouldn't necessarily take me at my word here.

Currently we fail flake8 pretty badly right now, but we can deal with that in another PR. This finishes up the list of objectives we had in #54. I feel content closing the issue once we actually start passing flake8, which I can get done when it isn't 1:30 am.

AndKaminer commented 8 months ago

For some reason when I run the tests, the pre-commit fails on windows, but not linux. All the other tests pass. Not sure what's up. I would put the output in here, but it's really long. Does whoever wrote the precommit config have any idea why that is?

aphedges commented 8 months ago

For some reason when I run the tests, the pre-commit fails on windows, but not linux. All the other tests pass. Not sure what's up. I would put the output in here, but it's really long. Does whoever wrote the precommit config have any idea why that is?

I was one of the people who worked on adding pre-commit, and both of us use Macs as dev machines, so we never encountered the problem on Windows. If you want help, you need to post the error. You can use the instructions on Organizing information with collapsed sections - GitHub Docs to include the error without it taking up too much space.

Without you saying anything, I suspect that your problem is with mixed-line-endings because Git changes line endings by default on Windows. You'll need to figure out how to override that during the checkout process.

AndKaminer commented 8 months ago

For some reason when I run the tests, the pre-commit fails on windows, but not linux. All the other tests pass. Not sure what's up. I would put the output in here, but it's really long. Does whoever wrote the precommit config have any idea why that is?

I was one of the people who worked on adding pre-commit, and both of us use Macs as dev machines, so we never encountered the problem on Windows. If you want help, you need to post the error. You can use the instructions on Organizing information with collapsed sections - GitHub Docs to include the error without it taking up too much space.

Without you saying anything, I suspect that your problem is with mixed-line-endings because Git changes line endings by default on Windows. You'll need to figure out how to override that during the checkout process.

Yes you're exactly right. I've been looking at it for the past little while. Sorry @aphedges ! I need to actually do a good hard look before posting. I went ahead and fixed the issue with mixed-line-endings, and now there's an issue with shellcheck. I'm taking a look at it now.

AndKaminer commented 8 months ago

Apparently shellcheck only works on linux when run through github actions. Not sure what to do about that. I'd like to tell it to run pre-commit with (SKIP=shellcheck) if it's running on windows and just precommit if on linux, but I can't seem to find a good way to do that.

aphedges commented 8 months ago

This fix looks like it will work: https://github.com/actions/checkout/issues/135. That way, you don't need to change mixed-line-ending.

AndKaminer commented 8 months ago

This fix looks like it will work: actions/checkout#135. That way, you don't need to change mixed-line-ending.

@aphedges That worked. Thank you! Sorry I couldn't figure that out myself.

AndKaminer commented 8 months ago

Okay, I think this is everything good now. I went ahead and added mac-os support to testing. I'll open up another PR to get rid of the deprecated methods in the example programs.

MarcCote commented 8 months ago

Looks good to me. I see all tests are passing on your repo's actions. Should be good. :+1: