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.

Sign up for GitHub

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

Jump to bottom

openlineage: tests: do not check whitespace in returned SQL statement after splitting it #40826

Merged
merged 2 commits into from
Jul 16, 2024
Merged

openlineage: tests: do not check whitespace in returned SQL statement after splitting it #40826

merged 2 commits into from
Jul 16, 2024

Conversation

mobuchowski
Copy link
Contributor

@mobuchowski mobuchowski commented Jul 16, 2024 •
edited
Loading

It's not important to check whether whitespace exactly matches in SQL statements after formatting it.

@mobuchowski
openlineage: strip returned SQL statements
97416c5 
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
@mobuchowski mobuchowski requested a review from potiuk July 16, 2024 19:40
@potiuk potiuk added the full tests needed We need to run full set of tests for this PR to merge label Jul 16, 2024
@potiuk potiuk closed this Jul 16, 2024
@potiuk potiuk reopened this Jul 16, 2024
@potiuk
Copy link
Member

potiuk commented Jul 16, 2024

Reopened with "full-tests-needed"

@potiuk
Copy link
Member

potiuk commented Jul 16, 2024

Still does not work :( same as my local

breeze testing tests --test-type "Providers[openlineage]" --force-lowest-dependencies

@mobuchowski
openlineage: do not check whitespace in SQL parser tests
40af8ac 
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
@mobuchowski mobuchowski changed the title openlineage: strip returned SQL statements openlineage: tests: do not check whitespace in returned SQL statement after splitting it Jul 16, 2024
@mobuchowski
Copy link
Contributor Author

@potiuk @vincbeck turns out sqlparse has some breaking changes for this between 0.5.0 and 0.5.1 releases.

We don't really care whether there's some spaces at the end of some statements - I rewrote this test to just compare the SQL strings with all whitespace removed - as Jarek suggested before.

@vincbeck
Copy link
Contributor

@potiuk @vincbeck turns out sqlparse has some breaking changes for this between 0.5.0 and 0.5.1 releases.

We don't really care whether there's some spaces at the end of some statements - I rewrote this test to just compare the SQL strings with all whitespace removed - as Jarek suggested before.

Looking good! Thanks :)

@vincbeck vincbeck merged commit 616c881 into main Jul 16, 2024
79 checks passed
@vincbeck vincbeck deleted the strip-overzealous-test branch July 16, 2024 22:14
@kacpermuda kacpermuda mentioned this pull request Jul 17, 2024
Merged
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
@mobuchowski @romsharon98
openlineage: tests: do not check whitespace in returned SQL statement…
aa00c88 
… after splitting it (apache#40826)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vincbeck vincbeck vincbeck approved these changes

@potiuk potiuk potiuk approved these changes

Assignees
No one assigned
Labels
area:providers full tests needed We need to run full set of tests for this PR to merge provider:openlineage AIP-53
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mobuchowski @potiuk @vincbeck