From 5bf3f6a818dd179cea4a84bd44df86ec1700ffc5 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 16 May 2023 10:57:20 +0200 Subject: [PATCH 1/2] workflow: produce a diff of the mypy errors before/after a PR A new job now runs on PRs. It will automatically run mypy on the target branch and on the source branch. It will generate a diff of the errors, showing the new ones and showing the ones that have been fixed. It will also show a summary with the number of errors before/after the PR. Because we have so many false positive, it makes no sense to mark this job red. So we always make it green (unless mypy can't run). As for now, it's up to developers to go check if any new error is introduced. If a line that used to produce a mypy error gets moved, it will be reported as: * a fixed error * a newly introduced error This is suboptimal and ideally we should have a way to detect moves. Signed-off-by: Olivier Gayot --- .github/workflows/build.yaml | 39 ++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index b553a1aa..eb60d361 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -30,3 +30,42 @@ jobs: - uses: actions/checkout@v2 - name: lint run: sudo ./scripts/test-in-lxd.sh ${{ matrix.image }} "make lint" + + static-typing: + # In this job, we compare the output of mypy before and after the PR. + if: github.event_name == 'pull_request' + runs-on: ubuntu-22.04 + steps: + - name: Install mypy and typeshed + run: sudo apt-get install -y python3-mypy python3-typeshed + - name: Checkout the pull request branch + uses: actions/checkout@v3 + with: + # When no ref is specified, a merge commit (from PR branch to target + # branch) will be checked out. Use the last commit of the PR branch + # instead. + ref: ${{ github.event.pull_request.head.sha }} + # By default, no ancestors of the specified revision will be checked + # out (see shallow repositories). Let's fetch just enough revisions + # so that, later, we can access the most recent common ancestor. + # If it does not work, we can set fetch-depth: 0 + fetch-depth: $(( ${{ github.event.pull_request.commits }} + 1 )) + - name: Run mypy on pull request branch + run: python3 -m mypy --ignore-missing-imports --check-untyped-defs subiquity subiquitycore system_setup console_conf scripts/replay-curtin-log.py | tee /tmp/mypy-head.out + - name: Determine base commit (most recent common ancestor) + id: determine_base_commit + run: | + ancestor=$(git merge-base -- \ + "${{ github.event.pull_request.base.sha }}" \ + "${{ github.event.pull_request.head.sha }}") + echo "base_commit=$ancestor" >> "$GITHUB_OUTPUT" + - name: Checkout the base commit (most recent common ancestor) + uses: actions/checkout@v3 + with: + ref: ${{ steps.determine_base_commit.outputs.base_commit }} + - run: git show + - name: Run mypy on base commit + run: python3 -m mypy --ignore-missing-imports --check-untyped-defs subiquity subiquitycore system_setup console_conf scripts/replay-curtin-log.py | tee /tmp/mypy-base.out + - name: Produce the diff between the two mypy runs + run: diff --color=always --unified=0 /tmp/mypy-base.out /tmp/mypy-head.out + continue-on-error: true From ac36ce5ddd61ec15925387bed74199e78611a09e Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Tue, 16 May 2023 12:45:41 +0200 Subject: [PATCH 2/2] filesystem: fix use of type hint for dictionary For a dictionary, the type hint should be dict[key_type, value_type] as opposed to dict[key_type: value_type]. Signed-off-by: Olivier Gayot --- subiquity/server/controllers/filesystem.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index d23f1843..91a9a79b 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -156,8 +156,8 @@ class FilesystemController(SubiquityController, FilesystemManipulator): self._core_boot_classic_error: str = '' self._source_handler: Optional[AbstractSourceHandler] = None self._system_mounter: Optional[Mounter] = None - self._role_to_device: Dict[str: _Device] = {} - self._device_to_structure: Dict[_Device: snapdapi.OnVolume] = {} + self._role_to_device: Dict[str, _Device] = {} + self._device_to_structure: Dict[_Device, snapdapi.OnVolume] = {} self.use_tpm: bool = False self.locked_probe_data: bool = False # If probe data come in while we are doing partitioning, store it in