Add FastAPI tests to Pydantic's CI tests #1075

tiangolo · 2019-12-04T12:13:27Z

Change Summary

Add FastAPI testing during Pydantic's CI tests.

It has a new Make target test-fastapi.

That, in turn, runs a script test-fastapi.sh.

The script will:

Clones FastAPI
Get the latest tag (the latest release)
Install it in the current environment
Install Pydantic from the source
Run FastAPI tests

I had to do it in an external Bash script after failing to do it inline in the Makefile. I couldn't find a way to create the variables latest_tag_commit and latest_tag to re-use them in the same Make target.

Related issue number

#1041

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
changes/<pull request or issue id>-<github username>.md file added describing change
(see changes/README.md for details)

…s in Make

codecov · 2019-12-04T12:29:30Z

Codecov Report

Merging #1075 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1075   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          20      20           
  Lines        3365    3365           
  Branches      663     663           
======================================
  Hits         3365    3365

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 890589a...7435b81. Read the comment docs.

samuelcolvin

This looks awesome.

Much simpler than I expected it to be, just a few small things.

samuelcolvin · 2019-12-04T12:19:37Z

.travis.yml

@@ -82,6 +82,7 @@ jobs:
    script:
    - pip uninstall -y cython email-validator typing-extensions devtools
    - make test
+    - make test-fastapi


I think fastapi tests should be be an entirely new stage.

Perfect. This should be corrected in the latest.

samuelcolvin · 2019-12-04T13:21:53Z

test-fastapi.sh

@@ -0,0 +1,24 @@
+#! /usr/bin/env bash


if you're brave you could probably implement this inside the Makefile.

Makefile even has it's own logic for checking if a file or directory exists.

If it's a pain, leave it for now and I'll fix in future.

otherwise maybe move this into tests to avoid cluttering up the root directory.

make the fastapi directory and any other related files should also live in tests.

About putting it in the Makefile, I tried first to put all the bash script in it, but I couldn't.

I tried several things, but I couldn't get the inline variables to work, the: latest_tag_commit and latest_tag. I also tried getting rid of them and "sub-bashing" the commands, nesting them, but couldn't get that to work either 😞

If you know a Make trick for that, let me know 😅

About the logic to check for the directory, I first had a:

fastapi: git clone https://github.com/tiangolo/fastapi.git .PHONY: test-fastapi test-fastapi: fastapi bash tests/test-fastapi.sh

To use the automatic directory target fastapi. But I removed it and put it all in the bash script, trying not to clutter the Makefile.

If you prefer it like that, with the git clone in the Makefile, I can put it there.

The rest of the Bash script... I don't really know what else to try to make those variables to work 🤔

I thought you would probably prefer the extra Make target, so I just went ahead and refactored that.

I moved the script to ./tests/.

But I couldn't move the fastapi repo clone to under tests, because tests is already a package, and it seems the imports get confused when trying to run tests inside of a directory inside of a tests directory.

I can also clone it to a ./tmp/fastapi dir to make it explicit. Or a ./.tmp/fastapi to make it a hidden dir.

samuelcolvin · 2019-12-04T13:23:18Z

test-fastapi.sh

+git checkout "${latest_tag}"
+pip install flit
+flit install
+SKIP_CYTHON=1 pip install -e ..


maybe this can be achieved by changing the recipe to require install as a parent-recipe?

I thought it wouldn't work, but it it did! 🎉

test-fastapi.sh

…rent source and make sure it actually breaks CI.

tiangolo · 2019-12-05T13:06:49Z

Thanks! 🚀 🎉

fix pydantic#1041 * Add test-fastapi.sh script, after failing to create reusable variables in Make * Add test-fastapi Make target * Add fastapi dir for tests to gitignore * Add fastapi tests to Travis * Update test-fastapi to run the minimum (pytest) * Move make test-fastapi to normal run, no "Without Deps" * Install Pydantic without Cython for FastAPI tests * Put FastAPI tests in its own Travis job * Add PR changes description * Remove coverage for FastAPI, it's not relevant for Pydantic * Implement code review changes, refactor Makefile use and fastapi tests * Add a bug intentionally, to test that FastAPI tests are using the current source and make sure it actually breaks CI. * Fix intentional bug. Confirmed it breaks Travis for FastAPI.

…ydantic#1075) Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com>

tiangolo added 6 commits December 4, 2019 13:05

Add test-fastapi.sh script, after failing to create reusable variable…

655c6a0

…s in Make

Add test-fastapi Make target

245822d

Add fastapi dir for tests to gitignore

7dadcc6

Add fastapi tests to Travis

3964313

Update test-fastapi to run the minimum (pytest)

f0cbcc6

Move make test-fastapi to normal run, no "Without Deps"

02a08f8

tiangolo added 4 commits December 4, 2019 13:32

Install Pydantic without Cython for FastAPI tests

3a858a3

Put FastAPI tests in its own Travis job

14fe357

Add PR changes description

b92563f

Remove coverage for FastAPI, it's not relevant for Pydantic

55cc52b

tiangolo requested review from samuelcolvin and dmontagu December 4, 2019 13:17

samuelcolvin reviewed Dec 4, 2019

View reviewed changes

tiangolo added 4 commits December 4, 2019 23:55

Implement code review changes, refactor Makefile use and fastapi tests

c19c4d5

Merge branch 'master' into fastapi-ci

d8b54ab

Add a bug intentionally, to test that FastAPI tests are using the cur…

f7f7af2

…rent source and make sure it actually breaks CI.

Fix intentional bug. Confirmed it breaks Travis for FastAPI.

7435b81

samuelcolvin merged commit ea709cc into pydantic:master Dec 5, 2019

tiangolo deleted the fastapi-ci branch December 5, 2019 13:06

alexdrydew pushed a commit to alexdrydew/pydantic that referenced this pull request Dec 23, 2023

Fix validation of Literal from JSON keys when used as dict key (p…

203b395

…ydantic#1075) Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add FastAPI tests to Pydantic's CI tests #1075

Add FastAPI tests to Pydantic's CI tests #1075

tiangolo commented Dec 4, 2019 •

edited

Loading

codecov bot commented Dec 4, 2019 •

edited

Loading

samuelcolvin left a comment

samuelcolvin Dec 4, 2019

tiangolo Dec 4, 2019

samuelcolvin Dec 4, 2019

samuelcolvin Dec 4, 2019

samuelcolvin Dec 4, 2019

tiangolo Dec 4, 2019

tiangolo Dec 4, 2019

tiangolo Dec 4, 2019

samuelcolvin Dec 4, 2019

tiangolo Dec 4, 2019

tiangolo commented Dec 5, 2019

Add FastAPI tests to Pydantic's CI tests #1075

Add FastAPI tests to Pydantic's CI tests #1075

Conversation

tiangolo commented Dec 4, 2019 • edited Loading

Change Summary

Related issue number

Checklist

codecov bot commented Dec 4, 2019 • edited Loading

Codecov Report

samuelcolvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiangolo commented Dec 5, 2019

tiangolo commented Dec 4, 2019 •

edited

Loading

codecov bot commented Dec 4, 2019 •

edited

Loading