CAT: Validate connector documentation #34380

darynaishchenko · 2024-01-19T16:58:01Z

What

resolved: #33750 #33749

How

Added TestConnectorDocumentation with:

test_prerequisites_content - test gets required fields from actual connector spec and check if title is in section under Prerequisites header. Special case is credentials. There are a lot of ways how to describe this step, so for credentials test searches for credentials keywords like auth, account etc.
test_docs_structure - test gets all heading from connector docs and check for the right order based on our standard template. If required heading is not in actual docs or not on a right order test will fail.
test_docs_descriptions - test gets headings that have specific description according our standard template and validates content after this heading to next one line by line. If actual documentation does have differences test will fail.
test_validate_links - retrieves all links from documentation file and makes a get request. if response status code in [200, 403, 401] it's a valid links, other links will be collected as invalid links and showed in assertation.

TestConnectorDocumentation is not required for now.

Recommended reading order

airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py
airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/conftest.py
airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/config.py
airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/doc_templates/ - folder with standard templates to compare with actual docs.

Manualy tested with:

source-google-ads
source-github
source-faker

vercel · 2024-01-19T16:58:08Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Feb 5, 2024 6:45pm

artem1205 · 2024-01-24T12:59:45Z

...ations/bases/connector-acceptance-test/unit_tests/data/docs/correct_all_description_exist.md

+
+Now that you have set up the GitHub source connector, check out the following GitHub tutorials:
+
+- [Creating PAT](https:github.com/how-to-create-a-pat)


Do we need another check for valid URL links? At least assertion for status_code

brianjlai

I have a feeling that this change might impact some of our existing connectors beyond what you tested, can we make sure to communicate to the team on how to fix these quickly if they fail standard validation?

Can we try to test this with a couple more certified connectors because we should try to be very diligent about how we validate.

Overall nice work so far, but I have a few things I flagged especially when we get very specific to some of the description validations

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

brianjlai · 2024-01-31T22:39:08Z

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

+            "For Airbyte Cloud:",
+            "For Airbyte Open Source:",
+            "Supported sync modes",
+            "Supported Streams",


This might not need to be required and we can relax this restriction. We have some cases like source-airtable which really functions more like a DB table hence we call it Supported Tables:

https://raw.githubusercontent.com/airbytehq/airbyte/master/docs/integrations/sources/airtable.md

I don't agree with that, airbyte protocol names it stream so docs should use this naming. Some users can be confused what they need streams or tables.

Yes you are right athatconsistent naming scheme is better. No need to address this part.

I did a check and the three sources that use Supported tables are my-hours, airtable, and dv-360. I think it is acceptable for these connector docs to fail and to be changed to Supported streams to pass tests.

brianjlai · 2024-01-31T22:42:01Z

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

+            connector_name,
+            "Prerequisites",
+            "Setup guide",
+            f"Set up {connector_name}",


Is this required and where? I did a quick look of google ads and while it is in the docs, its not a header and just in the text?

Am I missing something as to why this would pass the test?

docs criteria here, cell E35

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

brianjlai · 2024-01-31T23:21:56Z

airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/utils/docs.py

+        return heading.replace("Step 1: ", "")
+    if "Step 2: " in heading:
+        return heading.replace("Step 2: ", "")
+    return heading


why do we stop after Step 2? I presume that many connectors will have steps beyond 2.

thanks, really a good case with steps, as for example netsuite.md has more than 1 steps, we can try to strict compare step 1 and 2 as it is in connector standard template and skip others "Step" headers if they are after "Step 1:" header.

see other message about whether we need to test this

brianjlai · 2024-01-31T23:28:49Z

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

+            process = Thread(target=validate_docs_links, args=[link])
+            process.start()
+            threads.append(process)
+        [process.join() for process in threads]


can we add some sort of timeout or failsafe for this? I don't think we want to hold up a CAT run if for example some URLs end up hanging. Maybe 30 seconds (per thread) is a safe amount of time to wait and exit out if it's not fulfilled by then

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

brianjlai · 2024-01-31T23:51:49Z

...te-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

+                f"Documentation structure doesn't follow standard template. docs is not full.\nMissing headers: {template_headings[template_headings_index:]}"
+            )
+
+    def test_docs_descriptions(self, docs_path: str, connector_documentation: str, connector_metadata: dict):


Can we test this some additional certified connectors like source-greenhouse, source-airtable and a few others. I'm a little concerned how many connectors will fail this validation since it's quite specific to the exact text

Yes sure, I will test it with some more certified connectors and let you know the results. My previous tests, unfortunately , show differences in the actual and expected docs structure, but it was reasonable, errors like typos, incorrect order of heading etc were found using this test suite.

okay sounds good thank you

here is my test results for some connectors

I don't have access to this document. Please share with me

done, please let me know if you don't have access

darynaishchenko · 2024-02-01T10:33:33Z

I have a feeling that this change might impact some of our existing connectors beyond what you tested, can we make sure to communicate to the team on how to fix these quickly if they fail standard validation?

Yes, this test is really strict and most connectors might fail, but this test suite currently works as TestConnectorAttributes, it doesn't work unless connector_documentation is added to acceptance-test-config file.

…d steps, added timeout for test_validate_links

…ql lower than 400

darynaishchenko · 2024-02-05T18:57:18Z

Regarding a plan for the team how we will handle failures after this pr will be merged. I will create an issue with a list of cerified connectors and we will add a config for this test in acceptance-config.yaml for these connectors and fix docs in case test are failed. Now this test is not mandatory and will work only if config is provided in acceptance-config.yaml and ql > 400. Connectors without provided config will not be affected.

brianjlai

Please share the test document with me. But I think we can move forward with the test changes

darynaishchenko added 4 commits January 19, 2024 18:39

added TestConnectorDocumentation

0a26d13

added unittests

bcb8baa

added configuration for TestConnectorDocumentation

c4ecdbd

updated changelog+docs

a0134ce

darynaishchenko self-assigned this Jan 19, 2024

darynaishchenko requested review from lazebnyi and oustynova as code owners January 19, 2024 16:58

darynaishchenko requested a review from a team January 19, 2024 16:58

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Jan 19, 2024

darynaishchenko added 2 commits January 19, 2024 19:33

moved methods from test class to utils

143d633

Merge branch 'master' into daryna/cat/validate-source-docs

0c7c8f4

vercel bot deployed to Preview January 19, 2024 17:38 View deployment

format fix

e01d251

darynaishchenko requested review from bazarnov and artem1205 January 19, 2024 17:50

artem1205 reviewed Jan 24, 2024

View reviewed changes

lazebnyi requested a review from brianjlai January 25, 2024 13:36

added test_validate_links

be329a6

darynaishchenko requested a review from artem1205 January 29, 2024 12:50

artem1205 approved these changes Jan 29, 2024

View reviewed changes

darynaishchenko requested review from bnchrch and maxi297 January 29, 2024 15:02

brianjlai reviewed Jan 31, 2024

View reviewed changes

Merge branch 'master' into daryna/cat/validate-source-docs

de22cbe

vercel bot deployed to Preview February 1, 2024 11:49 View deployment

darynaishchenko added 3 commits February 5, 2024 19:55

added comments, updated test_docs_structure with removing non-require…

6ff070a

…d steps, added timeout for test_validate_links

updated unit tests

4ef7dd3

added operational_certification_test fixture to skip connector which …

c5e5df9

…ql lower than 400

darynaishchenko added 2 commits February 5, 2024 20:23

fix formatting

e175e2b

fixed unit tests

2ad3f78

darynaishchenko requested a review from brianjlai February 5, 2024 18:58

brianjlai approved these changes Feb 8, 2024

View reviewed changes

bazarnov approved these changes Feb 8, 2024

View reviewed changes

darynaishchenko merged commit 8107081 into master Feb 8, 2024
25 checks passed

darynaishchenko deleted the daryna/cat/validate-source-docs branch February 8, 2024 16:04

xiaohansong pushed a commit that referenced this pull request Feb 13, 2024

CAT: Validate connector documentation (#34380)

c8899a1

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 21, 2024

CAT: Validate connector documentation (airbytehq#34380)

1b21470

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024

CAT: Validate connector documentation (airbytehq#34380)

16d1bff

jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024

CAT: Validate connector documentation (airbytehq#34380)

868977f

xiaohansong pushed a commit that referenced this pull request Feb 27, 2024

CAT: Validate connector documentation (#34380)

a353fa3

darynaishchenko mentioned this pull request Jun 6, 2024

feat(qa_check): enable checking connector docs structure via qa check #39326

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CAT: Validate connector documentation #34380

CAT: Validate connector documentation #34380

darynaishchenko commented Jan 19, 2024 •

edited

Loading

vercel bot commented Jan 19, 2024 •

edited

Loading

artem1205 Jan 24, 2024

darynaishchenko Jan 29, 2024

brianjlai left a comment

brianjlai Jan 31, 2024

darynaishchenko Feb 5, 2024

brianjlai Feb 5, 2024

brianjlai Jan 31, 2024

darynaishchenko Feb 5, 2024

brianjlai Jan 31, 2024

darynaishchenko Feb 1, 2024

brianjlai Feb 5, 2024

brianjlai Jan 31, 2024

darynaishchenko Feb 5, 2024

brianjlai Jan 31, 2024

darynaishchenko Feb 1, 2024

brianjlai Feb 5, 2024

darynaishchenko Feb 5, 2024

brianjlai Feb 8, 2024

darynaishchenko Feb 8, 2024

darynaishchenko commented Feb 1, 2024

darynaishchenko commented Feb 5, 2024

brianjlai left a comment


		Now that you have set up the GitHub source connector, check out the following GitHub tutorials:

		- [Creating PAT](https:github.com/how-to-create-a-pat)

CAT: Validate connector documentation #34380

CAT: Validate connector documentation #34380

Conversation

darynaishchenko commented Jan 19, 2024 • edited Loading

What

How

Recommended reading order

Manualy tested with:

vercel bot commented Jan 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianjlai 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

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

darynaishchenko commented Feb 1, 2024

darynaishchenko commented Feb 5, 2024

brianjlai left a comment

Choose a reason for hiding this comment

darynaishchenko commented Jan 19, 2024 •

edited

Loading

vercel bot commented Jan 19, 2024 •

edited

Loading