Commit the generated .pyi files (#272)

* Commit the generated .pyi files

This should simplify some of our tests, as well as allow external tools
to run mypy on our repo (such as mypy_primer)

* Black test_negative as well

Author: nipunn1313

.gitattributes

@@ -1,3 +1,2 @@
-*.expected linguist-language=Python
 test_negative/output.expected.2.7 linguist-generated=true
 test_negative/output.expected.3.8 linguist-generated=true

.github/workflows/main.yml

@@ -95,7 +95,7 @@ jobs:
       - name: Run black
         run: |
           pip3 install black
-          black --check mypy_protobuf/main.py test/
+          black --check .
 
   sanity_check_windows:
     name: Sanity Check Windows Executable

.gitignore

@@ -8,7 +8,6 @@ __pycache__/
 /mypy_env/
 *.pyc
 /test/generated/**/*.py
-/test/generated/**/*.pyi
 !/test/generated/**/__init__.py
 .pytest_cache
 /build/

README.md

@@ -196,7 +196,7 @@ Contributions to the implementation are welcome. Please run tests using `./run_t
 Ensure code is formatted using black.
 ```
 pip3 install black
-black mypy_protobuf/main.py test/
+black .
 ```
 
 ## Contributors

pyproject.toml

@@ -4,3 +4,6 @@ requires = [
     "wheel",
 ]
 build-backend = "setuptools.build_meta"
+
+[tool.black]
+extend-exclude = "(_pb2.pyi?$|_pb2_grpc.pyi?$)"

run_test.sh

@@ -12,9 +12,6 @@ PY_VER_UNIT_TESTS="${PY_VER_UNIT_TESTS_3:=3.8.11} ${PY_VER_UNIT_TESTS_2:=2.7.18}
 PROTOC_ARGS="--proto_path=proto/ --experimental_allow_proto3_optional"
 GRPC_PROTOS=$(find proto/testproto/grpc -name "*.proto")
 
-# Clean out generated/ directory - except for .generated / __init__.py
-find test/generated -type f -not \( -name "*.expected" -or -name "__init__.py" \) -delete
-
 if [ -e $CUSTOM_TYPESHED_DIR ]; then
     export MYPYPATH=$CUSTOM_TYPESHED_DIR/stubs/protobuf
 fi
@@ -91,6 +88,11 @@ MYPY_PROTOBUF_VENV=venv_$PY_VER_MYPY_PROTOBUF
         exit 1
     fi
 
+    # CI Check to make sure generated files are committed
+    SHA_BEFORE=$(find test/generated -name "*.pyi" | xargs sha1sum)
+    # Clean out generated/ directory - except for __init__.py
+    find test/generated -type f -not -name "__init__.py" -delete
+
     # Compile protoc -> python
     $PROTOC $PROTOC_ARGS --python_out=test/generated `find proto -name "*.proto"`
 
@@ -108,6 +110,11 @@ MYPY_PROTOBUF_VENV=venv_$PY_VER_MYPY_PROTOBUF
 
     # Generate grpc protos
     $PROTOC $PROTOC_ARGS --mypy_grpc_out=test/generated $GRPC_PROTOS
+
+    if [[ -n $VALIDATE ]] && ! diff <(echo "$SHA_BEFORE") <(find test/generated -name "*.pyi" | xargs sha1sum); then
+        echo -e "${RED}Some .pyi files did not match. Please commit those files${NC}"
+        exit 1
+    fi
 )
 
 for PY_VER in $PY_VER_UNIT_TESTS; do
@@ -171,7 +178,7 @@ for PY_VER in $PY_VER_UNIT_TESTS; do
     )
 
     (
-        # Run unit tests. These tests generate .expected files
+        # Run unit tests.
         source $UNIT_TESTS_VENV/bin/activate
         if [[ $PY_VER =~ ^2.* ]]; then IGNORE="--ignore=test/test_grpc_usage.py"; else IGNORE=""; fi
         PYTHONPATH=test/generated py.test --ignore=test/generated $IGNORE -v

test/generated/google/protobuf/duration_pb2.pyi.expected renamed to test/generated/google/protobuf/duration_pb2.pyi

@@ -1,14 +1,11 @@
 """
-This file serves two purposes
-1) Run a test which ensures that mypy is producing the expected output. For
-developer convenience, in the case which it doesn't, it'll copy over the
-output to the expected output, so you can view the output diff in your
-code reviews. This will fail in CI if there is any inconsistency.
-
-2) This is a file which should mypy with success. See test_negative for
+This is a file which should mypy with success. See test_negative for
 a file that should have failures.
 
-Both of these tests are run by the run_test.sh script.
+This file contains a test to ensure that the test_negative failures
+are the expected failures
+
+These tests can be set up and run by the run_test.sh script
 """
 
 import glob
@@ -79,30 +76,6 @@ def _is_summary(l):
     return l.startswith("Found ") and l.endswith("source files)\n")
 
 
-def compare_pyi_to_expected(output_path):
-    # type: (str) -> Optional[str]
-    expected_path = output_path + ".expected"
-    assert os.path.exists(output_path)
-
-    with open(output_path) as f:
-        output_contents = f.read()
-
-    expected_contents = None  # type: Optional[str]
-    if os.path.exists(expected_path):
-        with open(expected_path) as f:
-            expected_contents = f.read()
-
-    if output_contents != expected_contents:
-        with open(expected_path, "w") as f:
-            f.write(output_contents)
-
-        return "{} doesn't match {}. This test will copy it over. Please rerun".format(
-            output_path, expected_path
-        )
-    else:
-        return None
-
-
 def test_generate_mypy_matches():
     # type: () -> None
     if sys.version_info < (3, 0):
@@ -129,18 +102,16 @@ def test_generate_mypy_matches():
 
         output = os.path.join("test", "generated", *components)
 
-        failure_check_results.append(compare_pyi_to_expected(output))
+        assert os.path.exists(output)
+        failure_check_results.append(output)
         if "grpc" in components:
             grpc_output = output[:-4] + "_grpc.pyi"
-            failure_check_results.append(compare_pyi_to_expected(grpc_output))
+            assert os.path.exists(grpc_output)
+            failure_check_results.append(grpc_output)
 
     # Make sure we checked everything
     assert len(failure_check_results) == len(pyi_files)
 
-    failures = ["\n\t" + f for f in failure_check_results if f]
-    if failures:
-        raise Exception("".join(failures))
-
 
 def test_generate_negative_matches():
     # type: () -> None
@@ -176,8 +147,8 @@ def grab_expectations(filename, marker):
     assert errors_38 == expected_errors_38
 
     # Some sanity checks to make sure we don't mess this up. Please update as necessary.
-    assert len(errors_27) == 60
-    assert len(errors_38) == 72
+    assert len(errors_27) == 62
+    assert len(errors_38) == 74
 
 
 def test_func():

test/generated/google/protobuf/empty_pb2.pyi.expected

@@ -155,7 +155,11 @@
 s8.email_by_uid[55] = "[email protected]"  # E:2.7 E:3.8
 s8.email_by_uid[UserId(55)] = "[email protected]"  # E:2.7 E:3.8
 s8.email_by_uid[55] = Email("[email protected]")  # E:2.7 E:3.8
-s8 = Simple1(user_id=55, email="[email protected]", email_by_uid={55: "[email protected]"})  # E:2.7 E:3.8
+s8 = Simple1(
+    user_id=55,  # E:2.7 E:3.8
+    email="[email protected]",  # E:2.7 E:3.8
+    email_by_uid={55: "[email protected]"},  # E:2.7 E:3.8
+)
 
 # Should not reexport inner.proto, since it doesn't have public tag.
 from testproto.reexport_pb2 import Inner  # E:2.7 E:3.8

test/generated/mypy_protobuf/extensions_pb2.pyi.expected renamed to test/generated/mypy_protobuf/extensions_pb2.pyi

@@ -50,58 +50,58 @@ def iter_requests() -> typing.Generator[DummyRequest, None, None]:
 
 class GoodServicer(DummyServiceServicer):
     def UnaryUnary(
-            self,
-            request: DummyRequest,
-            context: grpc.ServicerContext,
+        self,
+        request: DummyRequest,
+        context: grpc.ServicerContext,
     ) -> DummyReply:
         return DummyReply()
 
     def UnaryStream(
-            self,
-            request: DummyRequest,
-            context: grpc.ServicerContext,
+        self,
+        request: DummyRequest,
+        context: grpc.ServicerContext,
     ) -> typing.Iterator[DummyReply]:
         yield DummyReply()
 
     def StreamUnary(
-            self,
-            request: typing.Iterator[DummyRequest],
-            context: grpc.ServicerContext,
+        self,
+        request: typing.Iterator[DummyRequest],
+        context: grpc.ServicerContext,
     ) -> DummyReply:
         for data in request:
             pass
         return DummyReply()
 
     def StreamStream(
-            self,
-            request: typing.Iterator[DummyRequest],
-            context: grpc.ServicerContext,
+        self,
+        request: typing.Iterator[DummyRequest],
+        context: grpc.ServicerContext,
     ) -> typing.Iterator[DummyReply]:
         for data in request:
             yield DummyReply()
 
 
 class BadServicer(DummyServiceServicer):
     def UnaryUnary(  # E:3.8
-            self,
-            request: typing.Iterator[DummyRequest],
-            context: grpc.ServicerContext,
+        self,
+        request: typing.Iterator[DummyRequest],
+        context: grpc.ServicerContext,
     ) -> typing.Iterator[DummyReply]:
         for data in request:
             yield DummyReply()
 
     def UnaryStream(  # E:3.8
-            self,
-            request: typing.Iterator[DummyRequest],
-            context: grpc.ServicerContext,
+        self,
+        request: typing.Iterator[DummyRequest],
+        context: grpc.ServicerContext,
     ) -> DummyReply:
         for data in request:
             pass
         return DummyReply()
 
     def StreamUnary(  # E:3.8
-            self,
-            request: DummyRequest,
-            context: grpc.ServicerContext,
+        self,
+        request: DummyRequest,
+        context: grpc.ServicerContext,
     ) -> typing.Iterator[DummyReply]:
         yield DummyReply()