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

Jt400: possible missing resource in the native #6030

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

JiriOndrusek
Copy link
Contributor

fixes #6029

@zhfeng
Copy link
Contributor

zhfeng commented Apr 23, 2024

@JiriOndrusek is it possible to add an unit test?

@JiriOndrusek
Copy link
Contributor Author

@JiriOndrusek is it possible to add an unit test?

I'm thinking about that, I can try. Although the test would cover a scenario which does not make sense

@JiriOndrusek
Copy link
Contributor Author

@zhfeng test is added

@JiriOndrusek
Copy link
Contributor Author

@zhfeng I have to check again, whether the added test can not affect existing one when executed in different order

@JiriOndrusek JiriOndrusek force-pushed the jt400-native-resource branch 2 times, most recently from eb1e63f to 4d352f9 Compare April 23, 2024 12:51
@JiriOndrusek
Copy link
Contributor Author

@zhfeng I have to check again, whether the added test can not affect existing one when executed in different order

I run the tests ordered with new method at the first place and at last place and there was no issue.

@JiriOndrusek
Copy link
Contributor Author

JiriOndrusek commented Apr 23, 2024

@jamesnetherton As I think about the test coverage, it makes sense to me to move the test into jt400-mocked, because it does not require a real service and the test would be executed by CI (if the test is part of jt400-mocked.

Of course I have to try , whether it is possible). WDYT.

@jamesnetherton
Copy link
Contributor

move the test into jt400-mocked

Sounds ok to me. Or if we need to have it duplicated in both mocked and non-mocked, that's fine too.

@JiriOndrusek
Copy link
Contributor Author

move the test into jt400-mocked

Sounds ok to me. Or if we need to have it duplicated in both mocked and non-mocked, that's fine too.

Duplication (even if should be avoided) sounds like the right solution
.

@JiriOndrusek
Copy link
Contributor Author

I added the new test intojt400-mocked (as duplication)
Therefore it should be fulfilled that at leas one of jt400 and jt400-mocked is executed.

@jamesnetherton jamesnetherton merged commit 9d3bce3 into apache:main Apr 23, 2024
24 checks passed
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.

Jt400: possible missing resource in the native
3 participants