fix(binding-opcua;binding-coap) fix some test issues and resource leaks

[erossignon]

in particular, some resources leaks in coap and opcua test that prevents tests to terminate..

As a consequence it should be possible after this fix to run mocha without specifying --exit options to force termination.

[danielpeintner]

The tests fail

• not really sure why, see https://github.com/eclipse/thingweb.node-wot/pull/519/checks?check_run_id=3671865991
• coding style, please call npm run format

[relu91]

I am wondering why we didn't see any blocking issue before. 🤔 Even the latest PRs completed the test successfully.

Another note: it seems that this PR also solves eslint errors, but it is quite confusing since opc-ua is not actually included (yet) among eslint checked packages. In other words, it does not have the lint script in the package json, it should be added before fixing the eslint errors.

[erossignon]

lint issue:

My vscode is configured with the Prettier and eslint and highlights automatically the formatting or eslint error in the editor,
Error are easily spotted. it doesn't harm to fix some error now even if this project is not formally checked yet. we can add this in a different PR.

why problem was not spotted before ?

because mocha is configured to uses the --exit flag, and the --exit flag hides the problem. I will remove the --exit flag in a second phase.

[relu91]

Where is the exit flag?
https://github.com/eclipse/thingweb.node-wot/blob/1cd63abc520b9dcba96e3b88e348cbd83752f673/packages/binding-opcua/package.json#L51

https://github.com/eclipse/thingweb.node-wot/blob/1cd63abc520b9dcba96e3b88e348cbd83752f673/packages/binding-coap/package.json#L47

My vscode is configured with the Prettier and eslint and highlights automatically the formatting or eslint error in the editor,
Error are easily spotted. it doesn't harm to fix some error now even if this project is not formally checked yet. we can add this in a different PR.

Yeah also mine, but I would prefer to have consistency so if you don't mind I'd add the lint command in this PR.

[JKRhb]

It seems to me as if the CI got stuck in the tests for 06eb231. Maybe this run can be cancelled?

[danielpeintner]

It seems to me as if the CI got stuck in the tests for 06eb231. Maybe this run can be cancelled?

cancelled the workflow

[JKRhb]

Hmm, I am afraid the CI got stuck again.

[JKRhb]

Running the tests for this PR locally, it seems that the OPCUA client tests pass but the test suite doesn't terminate.

[erossignon]

Sorry, I put this PR into "draft" mode in order to prevent unnecessary reviewing while this is being still worked on.
I have excluded temporarily the attempt to fix the binding-opcua => this will come in a separate PR.

I will remove the "Draft" status when the PR will be ready for review.

[erossignon]

I have separated the going fix for the binding-opcua into an other PR.

[danielpeintner]

Most of the changes are in HTTP. @relu91 may I ask you to take a look?

[relu91]

I have couple of comments/suggestions after resolving those we can merge it 😃

[relu91]

Just to be sure, so the blocking issues were not in the HTTP binding right? I'm just concerned about the fact that we've never spotted it before.

[relu91]

One note: once we have fixed the CI issues here, we have to check if some of the coap open issues are solved by this.

[erossignon]

may I suggest that we use "rebase merging" as a default merge to master strategy to help creating a nice linear history in git ?

[danielpeintner]

LGTM

@relu91 ?

[danielpeintner]

may I suggest that we use "rebase merging" as a default merge to master strategy to help creating a nice linear history in git ?

I don't have a strong opinion.
I just noticed that @JKRhb squashes commits usually ... hence there seem to be different preferences.

[erossignon]

may I suggest that we use "rebase merging" as a default merge to master strategy to help creating a nice linear history in git ?

I don't have a strong opinion. I just noticed that @JKRhb squashes commits usually ... hence there seem to be different preferences.

Yes, squashing is also fine, although it can be made upfront on the developer side by doing a git rebase -i in his PR.
I am more about avoiding the merge commits that create complicated flow in the history.

[JKRhb]

I just noticed that @JKRhb squashes commits usually

That is a bit of a habit that I developed working with RIOT OS and their Git Workflow. They make extensive use of fixup! commits and the --autosquash flag of git rebase to keep the commit history clean.

may I suggest that we use "rebase merging" as a default merge to master strategy to help creating a nice linear history in git ?

If I see correctly, using the rebase option for merging does not create a merge commit at all, right? I would say having a reference on the main branch to PR the changes originated from would still be useful. But rebasing before the merge (with or without squashing) is definitely a good idea :)

[relu91]

LGTM

@relu91 ?

There is still one open comment from my previous review, but I can cover it myself. 👍🏻

About the merge strategy, I think this is not a good place to discuss it, I'll open an issue when we can explore different options/opinions.