Re-structured functional tests. #402

qlrd · 2025-03-07T14:34:25Z

Improves #252

What is the purpose of this pull request?

Which crates are being modified?

Description

This re-structure aims to optmize developer flow and the experience with tests tool through:

a replacement of the current python package management;
a more organized folder tree strucutre inside tests/; and
changes in tests/run_tests.py, tests/prepare.sh and tests/run.sh to
comply with the already mentioned changes.

The replacement in python package management is from poetry to uv (a
rust-based python management tool); this approach have some PROS:

built with rust;
faster than poetry;
cleanup some unecessary python dependencies;
more readable pyproject.toml;
compatible with python pip install -r approach (for old-school
pythonists).

Until now, a CON is:

this remove some poetry macros. Still seems like a good replacement.

The folder structure organization allows you to define test suites in
separate folders (e.g. "florestad", "floresta-cli") without needing to
define each of them in pyproject.toml. Files ending with "-test.py" in
each folder will be recognized and executed.

To comply with above mentioned changes, tests/run_tests.py was modified
to inlcude an argument --test-suite that will search for a folder
tests/<test-suite>. It was also necessary to change tests/prepare.sh
and tests/run.sh to replace poetry to uv.

The README helps the developer to:

install uv;
configure functional test environment;
format code;
lint code;
run tests.

It also updated .gitignore so it can help to avoid add .venv folder that
is created by uv tool.

Notes to the reviewers

A better context about this change is available in #252 (comment) and #391. Tested this approach in https://github.com/qlrd/Floresta/actions/runs/13722793641/job/38381971781

Checklist

I've signed all my commits
I ran just lint
I ran cargo test
I've checked the integration tests
I've followed the contribution guidelines
I'm linking the issue being fixed by this PR (if any)

Davidson-Souza

uv sounds promising, it really is faster than alternatives. If this indeed fixes our nix setup, more than happy to make this change.

Just some minor nits from me. I've ran the tests and works fine

README.md

tests/example/example-test.py

Optmize developer flow and the experience with tests tool through: * a replacement of the current python package management; * a more organized folder tree strucutre inside tests/; and * changes in tests/run_tests.py, tests/prepare.sh and tests/run.sh to comply with the already mentioned changes. The replacement in python package management is from poetry to uv (a rust-based python management tool); this approach have some PROS: * built with rust; * faster than poetry; * cleanup some unecessary python dependencies; * more readable pyproject.toml; * compatible with python pip install -r approach (for old-school pythonists). Until now, a CON is: * this remove some poetry macros. Still seems like a good replacement. The folder structure organization allows you to define test suites in separate folders (e.g. "florestad", "floresta-cli") without needing to define each of them in pyproject.toml. Files ending with "-test.py" in each folder will be recognized and executed. To comply with above mentioned changes, tests/run_tests.py was modified to inlcude an argument `--test-suite` that will search for a folder `tests/<test-suite>`. It was also necessary to change `tests/prepare.sh` and `tests/run.sh` to replace poetry to uv. The README helps the developer to: * install uv; * configure functional test environment; * format code; * lint code; * run tests. It also updated .gitignore so it can help to avoid add .venv folder that is created by uv tool.

Davidson-Souza

ACK 1beb0be

jaoleal · 2025-03-11T21:01:21Z

Im up with the changes but, before acking this i have to ask @Davidson-Souza about some CI logic that this Pr implies along with the changes needed for #391.

Should linting and formatting fail the test execution ? Youll see the commands to check lint and formatting are included in CI but outside prepare.sh and run.sh.

qlrd · 2025-03-11T22:42:43Z

Should linting and formatting fail the test execution ? Youll see the commands to check lint and formatting are included in CI but outside prepare.sh and run.sh.

This make sense in another workflow, as another checker, triggered before the functional.yml (like a lint-functional.yml), i.e, if the functional tests is not well formatted, don't make sense to run functional tests neither build floresta.

If it pass, go to functional.yml and anothers.

Davidson-Souza · 2025-03-11T23:33:38Z

Should linting and formatting fail the test execution ?

It should break CI. Perhaps getting the python lint check inside our linting task?

qlrd · 2025-03-12T11:51:15Z

Should linting and formatting fail the test execution ?

It should break CI. Perhaps getting the python lint check inside our linting task?

Maybe it will break CI regardless of where it is run if we don't do something like:

# .github/workflows/rust.yml
jobs:
  linting:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v4
      
      - name: Install uv
        uses: astral-sh/setup-uv@v5
        with:
          python-version: "3.12"
          enable-cache: true
          cache-dependency-glob: "uv.lock"

      - name: Prepare environment
        run: uv sync --all-extras --dev

      - name: Run black formatting
        continue-on-error: true # <----------------
        run: uv run black --check --verbose ./tests

      - name: Run pylint linter
        continue-on-error: true # <----------------
        run: uv run pylint --verbose ./tests

      - name: Install latest nightly
        uses: dtolnay/rust-toolchain@nightly
        with:
          components: rustfmt, clippy

      - name: Run cargo fmt
        run: cargo +nightly fmt --all --check

      - name: Run cargo clippy
        run: cargo +nightly clippy --all-targets -- -D warnings

This could be applied on functional.yml too, but with another step with a if: failure(), checking if the format/lint check failed and give some warning, or something similar, like a PR automated comment (but it will require some permission access)?

    - name: Run black formatting
      continue-on-error: true # <----------------
      run: uv run black --check --verbose ./tests

    - name: Comment on PR if black failed
      if: failure()
      uses: thollander/actions-comment-pull-request@v3
        with:
          message: |
            ...

Davidson-Souza · 2025-03-12T14:10:14Z

Maybe it will break CI regardless of where it is run if we don't do something like:

Yeah, the current way it will break. No reason to change IMO

jaoleal · 2025-03-12T15:55:19Z

No reason to change IMO

Okie Dokie

jaoleal

ACK 1beb0be

Davidson-Souza added dependencies Pull requests that update a dependency file code quality Generally improves code readability and maintainability functional tests labels Mar 7, 2025

qlrd force-pushed the uv-dep-management branch 2 times, most recently from 94d6357 to 9fef5fc Compare March 7, 2025 20:48

Davidson-Souza requested changes Mar 10, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Show resolved Hide resolved

README.md Show resolved Hide resolved

tests/example/example-test.py Outdated Show resolved Hide resolved

qlrd force-pushed the uv-dep-management branch 4 times, most recently from e87cb0b to 0d4618e Compare March 10, 2025 21:06

qlrd force-pushed the uv-dep-management branch from 0d4618e to 1beb0be Compare March 10, 2025 21:12

Davidson-Souza approved these changes Mar 10, 2025

View reviewed changes

Davidson-Souza requested a review from jaoleal March 10, 2025 23:31

jaoleal approved these changes Mar 12, 2025

View reviewed changes

Davidson-Souza merged commit 443bf8d into vinteumorg:master Mar 12, 2025
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Re-structured functional tests. #402

Re-structured functional tests. #402

Uh oh!

qlrd commented Mar 7, 2025 •

edited

Loading

Uh oh!

Davidson-Souza left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Davidson-Souza left a comment

Uh oh!

jaoleal commented Mar 11, 2025

Uh oh!

qlrd commented Mar 11, 2025

Uh oh!

Davidson-Souza commented Mar 11, 2025

Uh oh!

qlrd commented Mar 12, 2025 •

edited

Loading

Uh oh!

Davidson-Souza commented Mar 12, 2025

Uh oh!

jaoleal commented Mar 12, 2025 •

edited

Loading

Uh oh!

jaoleal left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Re-structured functional tests. #402

Re-structured functional tests. #402

Uh oh!

Conversation

qlrd commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of this pull request?

Which crates are being modified?

Description

Notes to the reviewers

Checklist

Uh oh!

Davidson-Souza left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Davidson-Souza left a comment

Choose a reason for hiding this comment

Uh oh!

jaoleal commented Mar 11, 2025

Uh oh!

qlrd commented Mar 11, 2025

Uh oh!

Davidson-Souza commented Mar 11, 2025

Uh oh!

qlrd commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Davidson-Souza commented Mar 12, 2025

Uh oh!

jaoleal commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaoleal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qlrd commented Mar 7, 2025 •

edited

Loading

qlrd commented Mar 12, 2025 •

edited

Loading

jaoleal commented Mar 12, 2025 •

edited

Loading

jaoleal left a comment •

edited

Loading