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

Fix tests on windows #2803

Merged
merged 4 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 25 additions & 13 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ jobs:
fail-fast: false
matrix:
include:
- {name: Windows, python: '3.12', os: windows-latest, tox: fail_fast_test_main}
# - {name: Mac, python: '3.12', os: macos-latest, tox: fail_fast_test_main}
- { name: "ruff", python: "3.11", os: ubuntu-latest, tox: "ruff" }
- { name: "mypy", python: "3.10", os: ubuntu-latest, tox: "mypy" }
- {name: Windows, python: '3.12', os: windows-latest, tox: fail_fast_test_main, skip_pre_build: "true" }
# - {name: Mac, python: '3.12', os: macos-latest, tox: fail_fast_test_main, skip_pre_build: "false" }
- { name: "ruff", python: "3.11", os: ubuntu-latest, tox: "ruff", skip_pre_build: "false" }
- { name: "mypy", python: "3.10", os: ubuntu-latest, tox: "mypy", skip_pre_build: "false" }
# run some integration tests and abort immediately if they fail, for faster feedback
- { name: "Linux", python: "3.12", os: ubuntu-latest, tox: fail_fast_test_main }
- { name: "3.12", python: "3.12", os: ubuntu-latest, tox: py312 }
- { name: "3.11", python: "3.11", os: ubuntu-latest, tox: py311 }
- { name: "3.10", python: "3.10", os: ubuntu-latest, tox: py310 }
- { name: "3.9", python: "3.9", os: ubuntu-latest, tox: py39 }
- { name: "Linux", python: "3.12", os: ubuntu-latest, tox: fail_fast_test_main, skip_pre_build: "false" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mquinnfd I think we could skip the pre build for the "fail fast" test on Ubuntu as well right? Maybe to keep things consistent with the windows test and it would probably run faster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So...

Sort of!?

I found that building the front end (or not) actually does affect the fail-fast tests, I could probably find an example from earlier - some things give a totally different response code though (200 vs 500) so my current understanding is that the UI build is actually not optional for those tests currently 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see and then I guess we just skip those tests if we're running on Windows

- { name: "3.12", python: "3.12", os: ubuntu-latest, tox: py312, skip_pre_build: "false" }
- { name: "3.11", python: "3.11", os: ubuntu-latest, tox: py311, skip_pre_build: "false" }
- { name: "3.10", python: "3.10", os: ubuntu-latest, tox: py310, skip_pre_build: "false" }
- { name: "3.9", python: "3.9", os: ubuntu-latest, tox: py39, skip_pre_build: "false" }

steps:
- uses: actions/checkout@v4
Expand All @@ -46,7 +46,7 @@ jobs:
- name: Install poetry plugin
run: python -m poetry self add "poetry-dynamic-versioning[plugin]"
# Install test dependencies (tox) to the root (non-package install)
- name: Install dependencies
- name: Install test dependencies
run: |
python -m poetry install --no-root --only test
- name: set full Python version in PY env var
Expand All @@ -60,11 +60,23 @@ jobs:
- name: Install Yarn
run: npm install -g yarn
# Build and install the package if it's Windows, to service shell-based tests
- if: ${{ matrix.os }} == "windows-latest"
- name: Install binary on Windows
if: ${{ matrix.os == 'windows-latest' }}
env:
SKIP_PRE_BUILD: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this is no longer needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're still doing a poetry build in here, so it will build the front end otherwise, we could of course tie this to the matrix value, so that it's picked up by any future Windows workflows (hopefully this step is removed soon though if I can figure out the venv stuff on the runner)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, I thought SKIP_PRE_BUILD was already set here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps SKIP_PRE_BUILD: ${{ matrix.skip_pre_build }} would make more sense?

# Make `locust` available on the Windows shell by installing it with `pip`
# before running tests which invoke it in the `cmd` prompt
run: |
python -m poetry build
pip install --find-links=dist locust
- run: python -m poetry run tox -e ${{ matrix.tox }}
python -m poetry self add "poetry-plugin-export"
python -m poetry export --without-hashes --format=requirements.txt > requirements.txt
pip install -r requirements.txt
pip install poetry-core>=1.0.0
pip install locust --find-links=dist
- name: Run tox tests
env:
SKIP_PRE_BUILD: ${{ matrix.skip_pre_build }}
andrewbaldwin44 marked this conversation as resolved.
Show resolved Hide resolved
run: python -m poetry run tox -e ${{ matrix.tox }}

lint_typecheck_test_webui:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ release: build
twine upload dist/*

setup_docs_dependencies:
poetry install --with docs
SKIP_PRE_BUILD=true poetry install --with docs

build_docs: setup_docs_dependencies
sphinx-build -b html docs/ docs/_build/
Expand Down
23 changes: 12 additions & 11 deletions docs/developing-locust.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,22 @@ Install Locust for development

Fork Locust on `GitHub <https://github.com/locustio/locust/>`_ and then run

.. note::
To build the Locust web UI, you must have `node` and `yarn` installed - see the *Making changes to Locust's Web UI* section below

.. code-block:: sh

# clone the repo
$ git clone git://github.com/<YourName>/locust.git

# install the poetry build system
$ pip3 install poetry
$ python -m pip install poetry

# install the dynamic versioning plugin for poetry
$ pip3 -m poetry self add "poetry-dynamic-versioning[plugin]"
$ python -m poetry self add "poetry-dynamic-versioning[plugin]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What will happen if this is not present? Can we make it optional, just to make the install easier for people? (maybe not now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In terms of the plugin?

The default versioning will be used, which is not that big a deal really, we could omit this from this section of the docs if it's easier 👍

You'll just end up with something like 0.0.1dev1 or similar as the package version, not a deal breaker if you're just wanting to build a dev version of the project

What do you think?


# perform an editable install of the "locust" package
$ pip3 -m poetry install --with dev
$ python -m poetry install --with dev,test

Now the ``locust`` command will run *your* code with no need for reinstalling after making changes.

Expand All @@ -40,8 +43,7 @@ We use `tox <https://tox.readthedocs.io/en/stable/>`_ to automate tests across m

.. code-block:: console

$ pip3 install tox
$ tox
$ python -m poetry run tox
...
py39: install_deps> python -I -m pip install cryptography mock pyquery retry
py39: commands[0]> python3 -m pip install .
Expand All @@ -53,7 +55,7 @@ To only run a specific suite or specific test you can call `pytest <https://docs

.. code-block:: console

$ pytest locust/test/test_main.py::DistributedIntegrationTests::test_distributed_tags
$ python -m pytest locust/test/test_main.py::DistributedIntegrationTests::test_distributed_tags

Formatting and linting
======================
Expand All @@ -62,15 +64,14 @@ Locust uses `ruff <https://github.com/astral-sh/ruff/>`_ for formatting and lint

.. code-block:: console

$ pip3 install ruff
$ python -m ruff --fix <file_or_folder_to_be_formatted>
$ python -m ruff format <file_or_folder_to_be_formatted>
$ python -m poetry run ruff --fix <file_or_folder_to_be_formatted>
$ python -m poetry run ruff format <file_or_folder_to_be_formatted>

You can validate the whole project using tox:

.. code-block:: console

$ tox -e ruff
$ python -m poetry run tox -e ruff
ruff: install_deps> python -I -m pip install ruff==0.1.13
ruff: commands[0]> ruff check .
ruff: commands[1]> ruff format --check
Expand All @@ -87,7 +88,7 @@ The documentation source is in the `docs/ <https://github.com/locustio/locust/tr

.. code-block:: console

$ pip3 -m poetry install --with docs
$ python -m poetry install --with docs

#. Build the documentation locally:

Expand Down
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ commands =
commands =
poetry install --with test
poetry run python -m unittest -f locust.test.test_main
setenv =
SKIP_PRE_BUILD = true
pass_env =
SKIP_PRE_BUILD

[testenv:ruff]
; Pin the same version in the .pre-commit-config.yaml file.
Expand Down
Loading