Issue #1891 #1899

AlexandrKhabarov · 2021-02-22T11:30:15Z

I have made things!

Add windows-specific build
Skip platform-specific tests

Checklist

I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
I have created at least one test case for the changes I have made
I have updated the documentation for the changes I have made
I have added my changes to the CHANGELOG.md

AlexandrKhabarov · 2021-02-22T12:24:18Z

@sobolevn
Hello, Nikita!
I'm not really sure that I properly understand how github actions actully works.
Can you review my request and give some feedback?

sobolevn

Thanks a lot! Please, take a look at my questions 👍

sobolevn · 2021-02-22T16:59:25Z

setup.cfg

@@ -127,6 +127,9 @@ rules =
  "sys_version_info < (3, 9)": py-lt-39
  "sys_version_info >= (3, 9)": py-gte-39

+  "exec('import os') or not os.name == 'nt'": not-win


We can reuse existing features from https://github.com/wemake-services/coverage-conditional-plugin/#writing-pragma-rules and https://www.python.org/dev/peps/pep-0496/#id9

sobolevn · 2021-02-22T17:00:18Z

.github/workflows/test.yml

+    runs-on: windows-latest
+    strategy:
+      matrix:
+        python-version: [ 3.6, 3.7, 3.8, 3.9 ]


Suggested change

python-version: [ 3.6, 3.7, 3.8, 3.9 ]

python-version: [3.6, 3.7, 3.8, 3.9]

sobolevn · 2021-02-22T17:00:29Z

.github/workflows/test.yml

+        with:
+          file: ./coverage.xml
+
+


Suggested change

sobolevn · 2021-02-22T17:01:48Z

tests/test_checker/test_noqa.py

@@ -24,6 +24,9 @@
 _PY38 = (3, 8) <= sys.version_info < (3, 9)
 _PY39 = (3, 9) <= sys.version_info < (3, 10)

+# Versions for different platform-specific fixtures.
+_WIN = os.name == 'nt'


See https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/logic/system.py#L11

And https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/logic/system.py#L3

sobolevn · 2021-02-22T17:04:35Z

wemake_python_styleguide/logic/system.py

    """Checks if a file is executable."""
    return os.access(filename, os.X_OK)


-def is_windows() -> bool:
+def is_windows() -> bool:  # pragma: win


This function executes on both linux and win code, so no reason to ignore it.

sobolevn · 2021-02-22T17:05:03Z

wemake_python_styleguide/visitors/tokenize/comments.py

@@ -219,7 +222,7 @@ def _check_executable_mismatch(
        token: tokenize.TokenInfo,
        *,
        is_shebang: bool,
-    ) -> None:
+    ) -> None:  # pragma: not-win


Can we move it to if is_windows() or self.filename == STDIN: line?

I think, we can not because testing shebang violations in this request was disabled.

sobolevn · 2021-02-22T17:05:16Z

wemake_python_styleguide/visitors/tokenize/comments.py

+    def visit_comment(
+        self,
+        token: tokenize.TokenInfo,
+    ) -> None:  # pragma: not-win


Do we really need this one?

sobolevn

Thanks a lot! Only a single question left!

sobolevn · 2021-02-23T12:50:42Z

.github/workflows/test.yml

+      - uses: crazy-max/ghaction-chocolatey@v1
+      - name: Install make
+        with:
+          args: install make


Can we merge curl and make together?

sobolevn · 2021-02-23T14:40:18Z

@AlexandrKhabarov please, take a look at https://github.com/AlexandrKhabarov/wemake-python-styleguide/actions/runs/592660378

sobolevn

YAML can be tricky! 😅

sobolevn · 2021-02-23T15:10:38Z

.github/workflows/test.yml

+    - uses: crazy-max/ghaction-chocolatey@v1
+    - name: Install curl and make


Suggested change

- uses: crazy-max/ghaction-chocolatey@v1

- name: Install curl and make

- name: Install curl and make

uses: crazy-max/ghaction-chocolatey@v1

I heve not used gitlab ci actions before. Currently, I can not understand the source of the problem. I can try to investigate it later.

codecov · 2021-02-23T21:16:19Z

Codecov Report

Merging #1899 (a38f140) into master (8f72647) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #1899   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          115       116    +1     
  Lines         6145      6182   +37     
  Branches      1375      1375           
=========================================
+ Hits          6145      6182   +37

Impacted Files	Coverage Δ
...ke_python_styleguide/visitors/tokenize/comments.py	`100.00% <ø> (ø)`
wemake_python_styleguide/logic/system.py	`100.00% <100.00%> (ø)`
wemake_python_styleguide/constants.py	`100.00% <0.00%> (ø)`

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 8f72647...a2b8d41. Read the comment docs.

But, I can google stuff: - https://github.com/marketplace/actions/install-poetry-action#windows

sobolevn · 2021-02-23T21:35:58Z

@AlexandrKhabarov can you please help me with fixing the CI? I don't have any experience with windows, it just scares me too much 😨

AlexandrKhabarov · 2021-02-24T08:46:19Z

I will try to solve it soon.

AlexandrKhabarov · 2021-02-24T15:50:24Z

I installed poetry and dependencies of the project, but now flake8 failed.

sobolevn · 2021-02-24T16:48:36Z

This is related to https://gitlab.com/pycqa/flake8-docstrings
AFAIK, it is not tested on windows, so it might have this problem.
Or we can skrew something up on ourside, you never know 🙂

Can you please report this issue to flake8-docstrings team? It looks like a valid issue to me 🤔

AlexandrKhabarov force-pushed the issue-1891 branch from 2d5189f to 3fb65a1 Compare February 22, 2021 11:49

AlexandrKhabarov changed the title ~~Issue 1891~~ Issue #1891 Feb 22, 2021

AlexandrKhabarov force-pushed the issue-1891 branch 2 times, most recently from 92c5ad5 to 86e8454 Compare February 22, 2021 11:56

sobolevn requested changes Feb 22, 2021

View reviewed changes

1891 skip platform-specific tests

8ba772e

AlexandrKhabarov force-pushed the issue-1891 branch from 86e8454 to b8eea45 Compare February 23, 2021 10:19

sobolevn reviewed Feb 23, 2021

View reviewed changes

AlexandrKhabarov force-pushed the issue-1891 branch from b8eea45 to b8635fb Compare February 23, 2021 13:19

AlexandrKhabarov force-pushed the issue-1891 branch from b8635fb to f066af8 Compare February 23, 2021 15:05

sobolevn reviewed Feb 23, 2021

View reviewed changes

1891 add build step for windows

c3f2276

AlexandrKhabarov force-pushed the issue-1891 branch from f066af8 to c3f2276 Compare February 23, 2021 15:20

sobolevn added 5 commits February 23, 2021 18:30

Update test.yml

1c5bbda

Update test.yml

82bf38e

Update test.yml

b7f6287

I don't know how windows works

8ce4b47

Fixes pragma marker

5293fec

sobolevn added 2 commits February 24, 2021 00:20

I still have no idea how windows works

949ec32

But, I can google stuff: - https://github.com/marketplace/actions/install-poetry-action#windows

Please, send help

1cf3615

1853 try to install poetry

a2b8d41

AlexandrKhabarov force-pushed the issue-1891 branch from 0eee189 to a2b8d41 Compare February 24, 2021 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #1891 #1899

Issue #1891 #1899

AlexandrKhabarov commented Feb 22, 2021

AlexandrKhabarov commented Feb 22, 2021

sobolevn left a comment

sobolevn Feb 22, 2021

AlexandrKhabarov Feb 23, 2021

sobolevn Feb 22, 2021

AlexandrKhabarov Feb 23, 2021

sobolevn Feb 22, 2021

AlexandrKhabarov Feb 23, 2021

sobolevn Feb 22, 2021

sobolevn Feb 22, 2021

AlexandrKhabarov Feb 23, 2021

sobolevn Feb 22, 2021

AlexandrKhabarov Feb 23, 2021

sobolevn Feb 22, 2021

AlexandrKhabarov Feb 23, 2021

sobolevn Feb 22, 2021

AlexandrKhabarov Feb 23, 2021

sobolevn left a comment

sobolevn Feb 23, 2021

AlexandrKhabarov Feb 23, 2021

sobolevn commented Feb 23, 2021

sobolevn left a comment

sobolevn Feb 23, 2021

AlexandrKhabarov Feb 23, 2021

codecov bot commented Feb 23, 2021 •

edited

Loading

sobolevn commented Feb 23, 2021

AlexandrKhabarov commented Feb 24, 2021

AlexandrKhabarov commented Feb 24, 2021

sobolevn commented Feb 24, 2021

	python-version: [ 3.6, 3.7, 3.8, 3.9 ]
	python-version: [3.6, 3.7, 3.8, 3.9]

		- uses: crazy-max/ghaction-chocolatey@v1
		- name: Install curl and make

Issue #1891 #1899

Are you sure you want to change the base?

Issue #1891 #1899

Conversation

AlexandrKhabarov commented Feb 22, 2021

I have made things!

Checklist

AlexandrKhabarov commented Feb 22, 2021

sobolevn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sobolevn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sobolevn commented Feb 23, 2021

sobolevn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 23, 2021 • edited Loading

Codecov Report

sobolevn commented Feb 23, 2021

AlexandrKhabarov commented Feb 24, 2021

AlexandrKhabarov commented Feb 24, 2021

sobolevn commented Feb 24, 2021

codecov bot commented Feb 23, 2021 •

edited

Loading