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

test(fix): refactor SQL query assertions in compoundNameConventionTest for accuracy #3277

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Jake-Zhi0Wang
Copy link

@Jake-Zhi0Wang Jake-Zhi0Wang commented Oct 4, 2024

The test failure is caused by a mismatch between the expected SQL query string and the actual SQL query string generated by .getSql() during the test. Specifically, the order of the selected columns in the SELECT DISTINCT clause can vary. Below is an example where it fails under NonDex on line 129

Click on to see more details on the error message when running this flaky test
[INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider
[INFO]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.google.cloud.spring.data.spanner.repository.query.SpannerStatementQueryTests
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.473 s <<< FAILURE! -- in com.google.cloud.spring.data.spanner.repository.query.SpannerStatementQueryTests
[ERROR] com.google.cloud.spring.data.spanner.repository.query.SpannerStatementQueryTests.compoundNameConventionTest -- Time elapsed: 1.445 s <<< FAILURE!
org.opentest4j.AssertionFailedError:

expected: "SELECT DISTINCT shares, trader_id, ticker, price, action, id, value FROM trades WHERE ( LOWER(action)=LOWER(@tag0) AND ticker=@tag1 ) OR ( trader_id=@tag2 AND price<@tag3 ) OR ( price>=@tag4 AND id IS NOT NULL AND trader_id=NULL AND trader_id LIKE @tag7 AND price=TRUE AND price=FALSE AND price>@tag10 AND price<=@tag11 AND price IN UNNEST(@tag12) AND value<@tag13 ) ORDER BY id DESC LIMIT 3"
 but was: "SELECT DISTINCT id, value, shares, price, ticker, action, trader_id FROM trades WHERE ( LOWER(action)=LOWER(@tag0) AND ticker=@tag1 ) OR ( trader_id=@tag2 AND price<@tag3 ) OR ( price>=@tag4 AND id IS NOT NULL AND trader_id=NULL AND trader_id LIKE @tag7 AND price=TRUE AND price=FALSE AND price>@tag10 AND price<=@tag11 AND price IN UNNEST(@tag12) AND value<@tag13 ) ORDER BY id DESC LIMIT 3"
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
        at com.google.cloud.spring.data.spanner.repository.query.SpannerStatementQueryTests.lambda$compoundNameConventionTest$0(SpannerStatementQueryTests.java:129)
        at com.google.cloud.spring.data.spanner.repository.query.SpannerStatementQueryExecutor.executeQuery(SpannerStatementQueryExecutor.java:95)
        at com.google.cloud.spring.data.spanner.repository.query.PartTreeSpannerQuery.executeRawResult(PartTreeSpannerQuery.java:73)
        at com.google.cloud.spring.data.spanner.repository.query.AbstractSpannerQuery.execute(AbstractSpannerQuery.java:63)
        at com.google.cloud.spring.data.spanner.repository.query.PartTreeSpannerQuery.execute(PartTreeSpannerQuery.java:35)
        at com.google.cloud.spring.data.spanner.repository.query.SpannerStatementQueryTests.compoundNameConventionTest(SpannerStatementQueryTests.java:179)
        at java.base/java.lang.reflect.Method.invoke(Method.java:569)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR]   SpannerStatementQueryTests.compoundNameConventionTest:179->lambda$compoundNameConventionTest$0:129
expected: "SELECT DISTINCT shares, trader_id, ticker, price, action, id, value FROM trades WHERE ( LOWER(action)=LOWER(@tag0) AND ticker=@tag1 ) OR ( trader_id=@tag2 AND price<@tag3 ) OR ( price>=@tag4 AND id IS NOT NULL AND trader_id=NULL AND trader_id LIKE @tag7 AND price=TRUE AND price=FALSE AND price>@tag10 AND price<=@tag11 AND price IN UNNEST(@tag12) AND value<@tag13 ) ORDER BY id DESC LIMIT 3"
 but was: "SELECT DISTINCT id, value, shares, price, ticker, action, trader_id FROM trades WHERE ( LOWER(action)=LOWER(@tag0) AND ticker=@tag1 ) OR ( trader_id=@tag2 AND price<@tag3 ) OR ( price>=@tag4 AND id IS NOT NULL AND trader_id=NULL AND trader_id LIKE @tag7 AND price=TRUE AND price=FALSE AND price>@tag10 AND price<=@tag11 AND price IN UNNEST(@tag12) AND value<@tag13 ) ORDER BY id DESC LIMIT 3"
[INFO]
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
[INFO]

To reproduce, run this at the root directory:

mvn -pl spring-cloud-gcp-data-spanner \
    edu.illinois:nondex-maven-plugin:2.1.7:nondex \
    -Dtest=com.google.cloud.spring.data.spanner.repository.query.SpannerStatementQueryTests#compoundNameConventionTest -DnondexRuns=10

(Note: The failing test might not be seen if every test happens to have the JSON string in the correct order. Try running it several times or increase the the number of runs with -DnondexRuns= to reproduce the issue.)

To address this, I refactored the SQL query assertions to avoid comparing the order of the SELECT clause fields, as .getSql() does not guarantee a specific order. I split the assertions into three parts to test the SQL query:

  • Ensured SQL query starts with "SELECT DISTINCT"
  • Extracted and verified SELECT clause fields in any order
  • Compared the the rest of SQL query with the expected formatted query

Please let me know if this approach works for you. If not, I'm happy to discuss alternatives and am willing to spend more time to address the test in the way you'd prefer. Thank you!

@Jake-Zhi0Wang Jake-Zhi0Wang requested a review from a team as a code owner October 4, 2024 00:44
Copy link

google-cla bot commented Oct 4, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

conventional-commit-lint-gcf bot commented Oct 4, 2024

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@Jake-Zhi0Wang Jake-Zhi0Wang changed the title Test Fix: Refactor SQL query assertions in test for accuracy in compoundNameConventionTest() test(fix): refactor SQL query assertions in compoundNameConventionTest for accuracy Oct 6, 2024
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.

1 participant