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

Standard tests: update template #2937

Merged
merged 18 commits into from
Apr 28, 2021
Merged

Conversation

keu
Copy link
Contributor

@keu keu commented Apr 18, 2021

closes #2185

What

Describe what the change is solving
It helps to add screenshots if it affects the frontend.

How

Describe the solution

Pre-merge Checklist

  • Run integration tests
  • Publish Docker images

Recommended reading order

  1. test.java
  2. component.ts
  3. the rest

@keu keu requested review from sherifnada, vitaliizazmic and yevhenii-ldv and removed request for jrhizor and ChristopheDuong April 18, 2021 18:57
@keu keu added the zazmic label Apr 18, 2021
@keu keu linked an issue Apr 18, 2021 that may be closed by this pull request
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

`we`` should also change the README to show how to run tests, and do the same thing for the singer template

@keu keu force-pushed the keu/standard-python-tests-gradle-cmd branch 2 times, most recently from 2243c25 to eae41d8 Compare April 19, 2021 18:06
Base automatically changed from keu/standard-python-tests-gradle-cmd to keu/standard-python-tests April 19, 2021 18:08
@keu keu force-pushed the keu/standard-python-tests-update-template branch from d5c6249 to 8a534bc Compare April 19, 2021 18:35
@keu keu requested a review from sherifnada April 19, 2021 19:53
Copy link
Contributor

@sherifnada sherifnada left a comment

Choose a reason for hiding this comment

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

Looks good with some small comments! We should merge this with the docs changes so they are done atomically probably.

@@ -86,6 +86,10 @@ docker run --rm -v $(pwd)/secrets:/secrets -v $(pwd)/sample_files:/sample_files
1. To run additional integration tests, place your integration tests in a new directory `integration_tests` and run them with `python -m pytest -s integration_tests`.
Make sure to familiarize yourself with [pytest test discovery](https://docs.pytest.org/en/latest/goodpractices.html#test-discovery) to know how your test files and methods should be named.

### Standard Tests
1. Update standard_test_config.yml file to disable/repeat or customize behaviour/inputs of specific tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. Update standard_test_config.yml file to disable/repeat or customize behaviour/inputs of specific tests.
1. Update standard_test_config.yml file to configure tests. See (standard testing)[some-link] for more information.

Copy link
Contributor

Choose a reason for hiding this comment

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

this link should point to the actual docs once we have them.

@@ -0,0 +1,5 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should get rid of the sample_files directory and just integration_tests or something. I think less people will be confused by this. I am in favor of doing the latter name. We can do it now or in another PR. up to you.

package_data={"": ["*.json"]}
install_requires=MAIN_REQUIREMENTS,
package_data={"": ["*.json"]},
extras_require={
Copy link
Contributor

Choose a reason for hiding this comment

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

is it idiomatic to require installing an extra to run tests? I wasn't sure if this was the right solution earlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there are many envs, testing env might require quite heavy dependencies, and absolutely not needed for a connector to work. usually, it is a good thing to separate such dependencies, as well as sometimes separate dev dependencies (like tools to debug, etc)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok in this case we should also update the README.md testing section to say that the user should install pip install ".[tests]" before running tests

@@ -99,6 +99,10 @@ docker run --rm -v $(pwd)/secrets:/secrets -v $(pwd)/sample_files:/sample_files
1. To run additional integration tests, create a directory `integration_tests` which contain your tests and run them with `pytest integration_tests`.
Make sure to familiarize yourself with [pytest test discovery](https://docs.pytest.org/en/latest/goodpractices.html#test-discovery) to know how your test files and methods should be named.

### Standard Tests
1. Update standard_test_config.yml file to disable/repeat or customize behaviour/inputs of specific tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as above

Base automatically changed from keu/standard-python-tests to master April 20, 2021 19:26
@keu keu force-pushed the keu/standard-python-tests-update-template branch from b8fb533 to 786f883 Compare April 22, 2021 13:44
@keu keu requested a review from sherifnada April 22, 2021 16:25
@keu keu mentioned this pull request Apr 22, 2021
* individual tests can be disabled in the config file
* individual tests can have different inputs and run multiple time with different sets of inputs
* each test has a timeout
* each test has stores all inputs and outputs
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* each test has stores all inputs and outputs
* each test stores all inputs and outputs, which can be used for debugging failures

@keu keu requested a review from sherifnada April 27, 2021 16:53
+ improve plugin behaviour, enable it explicitly
+ add acceptance.py to run acceptance tests together with integration tests
+ remove helper acceptance-test-python.sh
+ fix gradle command for acceptance tests
+ update docs
@keu keu force-pushed the keu/standard-python-tests-update-template branch from b3fa792 to dc689ab Compare April 28, 2021 20:17
@keu keu merged commit 88b77aa into master Apr 28, 2021
@keu keu deleted the keu/standard-python-tests-update-template branch April 28, 2021 22:11
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.

Standard Tests: Use by default in generated templates
3 participants