Skip to content

Add CP424 to Charset enum (Hebrew support) - #25

Open
roi0507 wants to merge 10 commits into
Blazemeter:masterfrom
openlegacy:master
Open

Add CP424 to Charset enum (Hebrew support)#25
roi0507 wants to merge 10 commits into
Blazemeter:masterfrom
openlegacy:master

Conversation

@roi0507

@roi0507 roi0507 commented Sep 10, 2025

Copy link
Copy Markdown

Great project!
Started doing dm3270 cleanup, until I found this library.
Will include the library in https://community.openlegacy.com/ol-terminal - OpenLegacy free web based terminal

Roi Mor, CTO

@Baraujo25

Copy link
Copy Markdown

@roi0507 Thank you so much for your interest in the project, I truly appreciate your contribution even more!


// Clean up - disconnect from the real mainframe
client.disconnect();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you for the effort you've put into adding these tests! However, I noticed that the tests currently rely on a real service connection. While this can provide some insights, it introduces potential flakiness and variability in our test results, which we want to avoid.

Additionally, the real service uses actual credentials that are likely stored in a property file, which for security reasons, I assume you did not include in the PR. This means that anyone trying to run these tests without those credentials will encounter failures.

In our project, we typically mock the service by recording the original flow and obfuscating sensitive information. This approach not only helps maintain consistent test results but also ensures that our tests run reliably every time.

Could we consider refactoring these tests to align with our mocking strategy?
You can follow any guide here on how to generate a dump file or see this video that walkthrough the process step by step: https://www.youtube.com/watch?v=Sr1BOi50xqg&t=2163s

We use Wiresham as our server mock tool. Once you have the dump, it's possible to obfuscate any sensible information presented either in the screen or as inputs.

I'm happy to help with this process!

Thank you again for your contributions!

@roi0507

roi0507 commented Oct 2, 2025

Copy link
Copy Markdown
Author

Hi @Baraujo25, Thanks for the details review. I meant to commit it to our forked repo only. You can ignore it, if you wish to accept the pull request.
Welcome you to try ol-terminal which uses this library :)

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.

5 participants