LewisGaul / minegauler

A clone of the original minesweeper game with many added features
GNU General Public License v3.0
11 stars 8 forks source link

fix: Test log to file tests.log #185

Closed ArtemErmulin closed 2 years ago

ArtemErmulin commented 2 years ago

According to issue #162

Pytest environ variable PYTEST_CURRENT_TEST is used.

LewisGaul commented 2 years ago

Hey Artem, I appreciate the contribution!

I'd like to suggest a different approach here. I strongly believe that mainline code should never have any logic just to help get the tests working - all test logic should be within the test code. Therefore I'm not happy with checking "PYTEST_CURRENT_TEST" in __main__.py, or specifying "test.log" there.

I would suggest the following approach:

How does that sound, would you be happy to make those changes? This would be a great quality-of-life improvement for the project!

codecov[bot] commented 2 years ago

Codecov Report

Merging #185 (f03bbfa) into dev (ffb5d4e) will increase coverage by 0.02%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #185      +/-   ##
==========================================
+ Coverage   70.71%   70.74%   +0.02%     
==========================================
  Files          38       38              
  Lines        3719     3719              
==========================================
+ Hits         2630     2631       +1     
+ Misses       1089     1088       -1     
Impacted Files Coverage Δ
minegauler/shared/types.py 86.87% <0.00%> (+0.62%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0d9590d...f03bbfa. Read the comment docs.

LewisGaul commented 2 years ago

Note that the branch to merge into is dev, not the default main branch - I've updated this for the PR, but you may wish to merge dev into yours as it's slightly ahead of main.

ArtemErmulin commented 2 years ago

Thanks, Lewis!

I overreacted a little while solving this problem, sorry) Thanks for the tip, I didn't know that pytest has such functionality. One moment, I'll prepare the changes.