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

Add conformance tests for transactional-expr #1054

Merged
merged 7 commits into from
Apr 29, 2022

Conversation

lasinicl
Copy link
Contributor

@lochana-chathura
Copy link
Member

@lasinicl are these passing locally? for me "28 tests completed, 22 failed"

@lasinicl
Copy link
Contributor Author

lasinicl commented Mar 7, 2022

@lasinicl are these passing locally? for me "28 tests completed, 22 failed"

I think they are failing with the test runner because transactions require ballerinai/transaction internal module and it's in a different repository.
cc: @pcnfernando

@pcnfernando
Copy link
Member

@lasinicl are these passing locally? for me "28 tests completed, 22 failed"

I think they are failing with the test runner because transactions require ballerinai/transaction internal module and it's in a different repository. cc: @pcnfernando

We have ballerinai/io as a dependency as its implementation is in the lang itself. But if we are to import ballerinai/transaction here; it will be a circular dependency. Let's have a offline discussion

a += 6;
}
} else {
check commit;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be an error? Since commit cannot be invoked outside a transactional mode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. we can probably do that. 🤔 It would be a breaking change though 😬

Copy link
Member

Choose a reason for hiding this comment

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

Can you create a issue please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pcnfernando
Copy link
Member

pcnfernando commented Mar 9, 2022

@lasinicl are these passing locally? for me "28 tests completed, 22 failed"

I think they are failing with the test runner because transactions require ballerinai/transaction internal module and it's in a different repository. cc: @pcnfernando

How did you manage to run the test-runner atm?

@lasinicl
Copy link
Contributor Author

lasinicl commented Mar 9, 2022

How did you manage to run the test-runner atm?

The test runner implementation is in a separate branch. https://github.com/ballerina-platform/ballerina-lang/tree/spec-conformance-test-runner

@pcnfernando
Copy link
Member

How did you manage to run the test-runner atm?

The test runner implementation is in a separate branch. https://github.com/ballerina-platform/ballerina-lang/tree/spec-conformance-test-runner

No, I meant the ballerinai/transaction depended ones?

@lasinicl
Copy link
Contributor Author

lasinicl commented Mar 9, 2022

No, I meant the ballerinai/transaction depended ones?

We can't really run these tests successfully using the test runner ATM due to this restriction.

@pcnfernando pcnfernando changed the title Add conformance tests for transactional-expr x Mar 9, 2022
@lasinicl lasinicl changed the title x Add conformance tests for transactional-expr Mar 9, 2022
@lasinicl lasinicl changed the base branch from conformance-tests to master April 19, 2022 03:15
@pcnfernando
Copy link
Member

pcnfernando commented Apr 27, 2022

@chiranSachintha @lasinicl merging this will break the test runner in lang; shall we skip transactional-expr label in the test-runner pointing to ballerina-platform/ballerina-lang#35939 until we support it

@lasinicl lasinicl requested review from lochana-chathura and removed request for rpjayasekara April 27, 2022 08:01
@lasinicl
Copy link
Contributor Author

@chiranSachintha @lasinicl merging this will break the test runner in lang; shall we skip transactional-expr label in the test-runner pointing to ballerina-platform/ballerina-lang#35939 until we support it

Will be handled via ballerina-platform/ballerina-lang#35943

@pcnfernando pcnfernando merged commit 0ac38a1 into ballerina-platform:master Apr 29, 2022
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.

Write Spec Conformance Tests for transactional expression
3 participants