search

wallento / wavedrompy

WaveDrom compatible python command line
Other
97 stars 23 forks source link

Regression Tests for Issue #7, Issue #11, Issue #10, and Issue #13 #15

Closed MutantPlatypus closed 5 years ago

MutantPlatypus commented 5 years ago

Hi @wallento. I made three JSONs for the issues I reported and changed how the issue regression tests were specified: It think it's clearer to just match them up directly with their GitHub Issue number.

wallento commented 5 years ago

Hi,

I had the same though initially, but I wanted to wait with naming them depending on whether the original repo moves to get under my control and then close this hard fork. Issue #s won't match then. But anyhow, lets start this way for now.

Unfortunately, tests don't pass. It seems minor issues, but I want to fix them before merging.

Thanks a lot for taking the time!

wallento commented 5 years ago

Hm, they are not failing? Strange, I actually had one local yesterday and it failed and then decided to go to bed instead :)

MutantPlatypus commented 5 years ago

Thanks! My first pull request, merged!

Hm, they are not failing? Strange, I actually had one local yesterday and it failed...

Yeah, I was just relying on Travis CI to do the tests for me since I didn't know how to do them locally. I was waiting to branch until you had a commit that was passing tests according to Travis CI.

I'll try what you posted in Issue #7:

I am rather conservative:

  • Create multiple virtual envs for different Python versions (I run them occasionally on Python 2.7 and Python 3.5)
  • pip install -e .[test] in each venv
  • WAVEDROMDIR=path/to/wavedrom pytest -v in each venv (mostly -k for some specific test I am fixing)

Is there a bug in your local tests? Should I wait until you find and fix it?

wallento commented 5 years ago

Congrats!

Everything works smoothly now, I pushed another small change so that it just reads files from the directory listing into the lists.