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

Fix stuck merge process by correcting input stream validation Issue #2953 #2987

Merged
merged 2 commits into from Aug 16, 2022

Conversation

phanlezz
Copy link
Collaborator

@phanlezz phanlezz commented Aug 15, 2022

Fix for importers who were stuck on merge process

This fixes a typo that caused the parsing to fail on nearly every OS, on different terminals, depending on, if they send an EOF token or not.
The typo was introduced during the compatibility conversion to java8: #2930
This available check is required, because of a workaround described here #2854

Root of the problem is that we always wait for stdin: #2858

Issue: #2953

#2953
This fixes a bug in the workaround for the input stream validation, by removing a typo
@ce-bo
Copy link
Collaborator

ce-bo commented Aug 15, 2022

This Bug should have broken out Pipeline. Would you please Check golden_test.sh and extend the script to have a regression test? Thank you!

@sonarcloud
Copy link

sonarcloud bot commented Aug 15, 2022

[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@sonarcloud
Copy link

sonarcloud bot commented Aug 15, 2022

[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@phanlezz
Copy link
Collaborator Author

This Bug should have broken out Pipeline. Would you please Check golden_test.sh and extend the script to have a regression test? Thank you!

I think we should merge this for now as-is. It is difficult to build tests that simulate an open input stream on a command line.
We need to fix our behavior for input files. An issue is already open for this.

@phanlezz phanlezz merged commit f741b0a into main Aug 16, 2022
@phanlezz phanlezz deleted the fix/2953/parsing-fails-everywhere branch August 16, 2022 08:29
@BridgeAR
Copy link
Member

I think we should merge this for now as-is. It is difficult to build tests that simulate an open input stream on a command line.
We need to fix our behavior for input files. An issue is already open for this.

Merging this as is means the test is going to be deprioritized and that seems quite urgent.
We can execute the command and check that it's successful and the output file is generated. That would be a sufficient test. The file would have to be created in a temp file with a random name and should be deleted after the test.

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

4 participants