Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split the self_test part of the tests into another file #363

Merged
merged 1 commit into from
Sep 4, 2023

Conversation

zz1874
Copy link
Collaborator

@zz1874 zz1874 commented Sep 1, 2023

This PR is a fix of self_test (#123) depending on #362.

The self_test always failed when we added the functionality of parsing typeshed packages like types-setuptools because it wasn't installed in our virtual environment of CI.

In our tests.yaml, we have

     - name: Install project
        run: poetry install --no-interaction --sync --with=nox,test

before

      - name: Run fawltydeps on Python ${{ matrix.python-version }}
        run: poetry run nox --no-venv --no-install --non-interactive -s self_test -- -vv

while running fawltydeps against itself needs to install all optional dependencies, including "nox", "test", "lint", "format", "dev", and types-setuptools is a dependency of lint.

We cannot just install the projects with all optional dependencies in tests.yaml because in our unit tests, we checked click package as an example, which will be installed when including format, and then the tests/test_install_deps.py::test_resolve_dependencies_install_deps__via_local_cache will fail because it will use LocalPackageResolver instead of TemporaryPipInstallResolver. (The detailed logs are here)

So, after thinking of this, splitting the self_test part of the tests may be a better solution. In this way, in the self test, we install all dependencies and optional dependencies and do a complete test, and in the normal tests, we do the remaining tests with only "nox" and "test" groups installed.

@zz1874 zz1874 force-pushed the zhihan/fix-self-test-in-CI branch 3 times, most recently from e138a5b to 14de4b3 Compare September 1, 2023 15:26
@zz1874 zz1874 marked this pull request as ready for review September 1, 2023 15:30
@zz1874 zz1874 requested review from Nour-Mws and mknorps September 1, 2023 15:30
Copy link
Collaborator

@Nour-Mws Nour-Mws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for figuring this up @zz1874! This was not straightforward to debug.
I took a little pause on the inclusion of all deps groups in the nox session as I imagine lint would be enough. But then reasoned that since we're testing FawltyDeps as a project, it makes sense to include all deps groups in pyproject.toml in the environment we're using to resolve the deps.
Could you please reduce this PR to the last commit only? We can rebase PR #362 once that's done and re-run the CI there.

@zz1874 zz1874 force-pushed the zhihan/fix-self-test-in-CI branch from 14de4b3 to 7a3183f Compare September 4, 2023 11:27
@zz1874 zz1874 merged commit 19f370c into main Sep 4, 2023
@zz1874 zz1874 deleted the zhihan/fix-self-test-in-CI branch September 4, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants