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

Added tox config with flake8 and nosetests. #6

Merged
merged 9 commits into from
May 14, 2015
Merged

Added tox config with flake8 and nosetests. #6

merged 9 commits into from
May 14, 2015

Conversation

tmatsuo
Copy link
Contributor

@tmatsuo tmatsuo commented May 8, 2015

No description provided.

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 8, 2015

@elibixby I'd appreciate it if you could take a look.

@tmatsuo tmatsuo mentioned this pull request May 8, 2015
@@ -1,6 +1,6 @@
# dummy app.yaml for nosegae

application: ndb-snippets
application: python-docs-samples
Copy link
Contributor

Choose a reason for hiding this comment

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

application and version field are supposed to be deprecated yes? Could we move these to env vars?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, we don't need those values

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 8, 2015

Thanks, PTAL

[run]
include =
datastore/*
localtesting/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Merging #2 So please add bigquery/

@elibixby
Copy link
Contributor

elibixby commented May 8, 2015

Besides comment LGTM

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 8, 2015

Well, this time I'm going to add bigquery, but it shows 100 lines of flake8 errors (and I need to fix them).
Can you stop merging new directory before merging the tox config?

@elibixby
Copy link
Contributor

elibixby commented May 8, 2015

Yeah Sorry! That's weird I ran flake on it right before merging and it was fine! (oh except I ran the licensing tool after that, I bet those 100 lines of errors are in the licenses...)

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 8, 2015

Yes, turns out that it's not a big problem

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 8, 2015

Ugh, that's not limited to licenses. Anyways, I'll try to fix them

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 8, 2015

Ok, the tests are all green when run locally.
Let's see if the travis build suceeds.

@elibixby
Copy link
Contributor

elibixby commented May 8, 2015

Might have to wait until after the weekend... =/ Travis builds have been super slow to start lately.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling ac005e7 on tox into * on master*.

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 9, 2015

All green :)

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 11, 2015

@elibixby
I addressed all of your comments, and tests are all green now.
Can I merge this now?

@elibixby
Copy link
Contributor

Sorry! Was on vacation! LGTM, merge away.

@tmatsuo
Copy link
Contributor Author

tmatsuo commented May 14, 2015

No worries. Thanks for looking!

tmatsuo pushed a commit that referenced this pull request May 14, 2015
Added tox config with flake8 and nosetests.
@tmatsuo tmatsuo merged commit 32fd8ba into master May 14, 2015
@tmatsuo tmatsuo deleted the tox branch May 14, 2015 17:11
dvioreltx pushed a commit to dvioreltx/python-docs-samples that referenced this pull request May 27, 2020
gcf-merge-on-green bot pushed a commit that referenced this pull request Apr 20, 2022
## Description

#7342 introduced a change where nox would check for `test_*.py` and `*_test.py` files. Django and IDP-SQL had their tests in `test/`, so this glob didn't find these files, and these tests haven't been run for a bit. 



Outstanding issues with these tests: 

*dynamic PR status update follows*

---


 * Django: Postgres 9.6 EOL

```
Step #6 - "Migrate database": django.db.utils.ProgrammingError: column c.relispartition does not exist
Step #6 - "Migrate database": LINE 3:             CASE WHEN c.relispartition THEN 'p' WHEN c.relki...
Step #6 - "Migrate database": 
```

Debugging seems to indicate this happens on old versions of postgres. [Django 4.0 no longer supports Postgres 9.6](https://docs.djangoproject.com/en/4.0/releases/4.0/#dropped-support-for-postgresql-9-6). `test-instance-pg` is Postgres 9.6, which is EOL as of November 2021. 

Suggest creating a newer versioned instance of a postgres testing database (happy to help here where I can).

Update: test-instance-pg has been updated to Postgres 14

---

* IDP: Linting issues

```
./middleware.py:31:29: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                            ^
./middleware.py:31:42: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                                         ^
./middleware.py:31:61: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                                                            ^
```

This is being addressed as of v2.9.0 of flake8-annotations. sco1/flake8-annotations#136 

It will require adding `--allow-star-arg-any` to the flake8 call. 

Update: while this flag could be used, I opted to use the `TypeVar` method in #7789

---

## Checklist
- [ ] I have followed [Sample Guidelines from AUTHORING_GUIDE.MD](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md)
- [ ] README is updated to include [all relevant information](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#readme-file)
- [ ] **Tests** pass:   `nox -s py-3.6` (see [Test Environment Setup](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup))
- [ ] **Lint** pass:   `nox -s lint` (see [Test Environment Setup](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup))
- [ ] These samples need a new **API enabled** in testing projects to pass (let us know which ones)
- [ ] These samples need a new/updated **env vars** in testing projects set to pass (let us know which ones)
- [ ] Please **merge** this PR for me once it is approved.
- [ ] This sample adds a new sample directory, and I updated the [CODEOWNERS file](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/.github/CODEOWNERS) with the codeowners for this sample
leahecole pushed a commit that referenced this pull request May 25, 2022
## Description

#7342 introduced a change where nox would check for `test_*.py` and `*_test.py` files. Django and IDP-SQL had their tests in `test/`, so this glob didn't find these files, and these tests haven't been run for a bit. 



Outstanding issues with these tests: 

*dynamic PR status update follows*

---


 * Django: Postgres 9.6 EOL

```
Step #6 - "Migrate database": django.db.utils.ProgrammingError: column c.relispartition does not exist
Step #6 - "Migrate database": LINE 3:             CASE WHEN c.relispartition THEN 'p' WHEN c.relki...
Step #6 - "Migrate database": 
```

Debugging seems to indicate this happens on old versions of postgres. [Django 4.0 no longer supports Postgres 9.6](https://docs.djangoproject.com/en/4.0/releases/4.0/#dropped-support-for-postgresql-9-6). `test-instance-pg` is Postgres 9.6, which is EOL as of November 2021. 

Suggest creating a newer versioned instance of a postgres testing database (happy to help here where I can).

Update: test-instance-pg has been updated to Postgres 14

---

* IDP: Linting issues

```
./middleware.py:31:29: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                            ^
./middleware.py:31:42: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                                         ^
./middleware.py:31:61: ANN401 Dynamically typed expressions (typing.Any) are disallowed
    def decorated_function(*args: Any, **kwargs: Any) -> Any:
                                                            ^
```

This is being addressed as of v2.9.0 of flake8-annotations. sco1/flake8-annotations#136 

It will require adding `--allow-star-arg-any` to the flake8 call. 

Update: while this flag could be used, I opted to use the `TypeVar` method in #7789

---

## Checklist
- [ ] I have followed [Sample Guidelines from AUTHORING_GUIDE.MD](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md)
- [ ] README is updated to include [all relevant information](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#readme-file)
- [ ] **Tests** pass:   `nox -s py-3.6` (see [Test Environment Setup](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup))
- [ ] **Lint** pass:   `nox -s lint` (see [Test Environment Setup](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/AUTHORING_GUIDE.md#test-environment-setup))
- [ ] These samples need a new **API enabled** in testing projects to pass (let us know which ones)
- [ ] These samples need a new/updated **env vars** in testing projects set to pass (let us know which ones)
- [ ] Please **merge** this PR for me once it is approved.
- [ ] This sample adds a new sample directory, and I updated the [CODEOWNERS file](https://github.com/GoogleCloudPlatform/python-docs-samples/blob/main/.github/CODEOWNERS) with the codeowners for this sample
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.

3 participants