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

@W-14371508 fix: JS Substitute DB tests for parity #142

Conversation

ashwanikmr121
Copy link
Contributor

Add missing DB tests for the Substitute function to bring it to parity with Postgres tests including the JS path

@ashwanikmr121
Copy link
Contributor Author

ashwanikmr121 commented Mar 14, 2024

@sahil-here As discussed, added tests to other DB files for parity purposes. However, I wanted to check If we expect the relevant gold files for other DBs to also be generated as there are none generated for me like it was generated for Postgres in the PR: #140.

Probably the gold files are not changed as the DB tests other than Postgres do not include an execution path for Javascript where the code changes have gone in.

@ashwanikmr121 ashwanikmr121 merged commit 6092e00 into salesforce:main Mar 15, 2024
2 checks passed
@ashwanikmr121 ashwanikmr121 deleted the ashwani-add-missing-db-tests-substitute branch March 15, 2024 06:55
@sahil-here
Copy link
Collaborator

sahil-here commented Mar 16, 2024

s to other DB files for parity purposes. However, I wanted to check If we expect the relevant gold files for other DBs to also be generated as there are none generated for me like it was generated for Postgres in the PR: #140.

The goldfiles for other DBs will not change as the goldfiles only contain generated SQL for execution path sql. And you are right, the goldfile for Postgres changed, as it has an execution path javascript, which will generate the Javascript code in the goldfile which has changed based on the PR.

Also, if you want to run all tests for all DB substrates, you can use a command like: mvn -P db-tests test to run these db tests.

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.

None yet

2 participants