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.

Sign up for GitHub

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

Jump to bottom

test(connector-polkadot): fix broken tests (all) due to ESM imports #3077

Closed
petermetz opened this issue Mar 11, 2024 · 2 comments · Fixed by #3417
Closed

test(connector-polkadot): fix broken tests (all) due to ESM imports #3077

petermetz opened this issue Mar 11, 2024 · 2 comments · Fixed by #3417
Assignees
@petermetz
Labels
Flaky-Test-Automation Issues related to test stability (which is a long running issue that can never fully be solved) Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label. P3 Priority 3: Medium Polkadot Tasks related or relevant to the Polkadot connector Tests Anything related to tests be that automatic or manual, integration or unit, etc.
Milestone
v2.0.0

Comments

@petermetz
Copy link
Contributor

Description

I remember running the tests of the polkadot PR at some point and them passing, but now they appear as if they were always broken /facepalm.

Logs:
2024-02-27T21-38-00-cacti-polkadot-connector-test-failures.log

@AnmolBansalDEV Do you remember when the tests broke and what change triggered it?

Acceptance Criteria

  1. Tests are passing
@petermetz petermetz added Polkadot Tasks related or relevant to the Polkadot connector P3 Priority 3: Medium Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label. Flaky-Test-Automation Issues related to test stability (which is a long running issue that can never fully be solved) Tests Anything related to tests be that automatic or manual, integration or unit, etc. labels Mar 11, 2024
@outSH
Copy link
Contributor

outSH commented Mar 18, 2024

@petermetz I think when you add new tests to CI they do not run in that particular PR (i.e. CI file is taken from main, not the branch that is being merged), and this suite has been failing from the beginning.

@petermetz
Copy link
Contributor Author

@petermetz I think when you add new tests to CI they do not run in that particular PR (i.e. CI file is taken from main, not the branch that is being merged), and this suite has been failing from the beginning.

@outSH Damn. Sorry I missed that and thank you for noticing this hole in our review process.

@petermetz petermetz self-assigned this Jul 15, 2024
@petermetz petermetz added this to the v2.0.0 milestone Jul 15, 2024
petermetz added a commit to petermetz/cacti that referenced this issue Jul 15, 2024
@petermetz
fix(connector-polkadot): use dynamic import calls for ESM dependencies
b5cbbd9 
1. The polkadot libraries are all ESM-only so we have to import them
in a specific way in order to avoid runtime crashes.
2. This was not done in the original implementation of the polkadot
connector and because of that all the tests were failing.
3. Refactoring the code so that all polkadot related dependencies are
imported dynamically fixes the issue.

Fixes hyperledger-cacti#3077

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
petermetz added a commit to petermetz/cacti that referenced this issue Jul 16, 2024
@petermetz
fix(connector-polkadot): use dynamic import calls for ESM dependencies
cf2727d 
1. The polkadot libraries are all ESM-only so we have to import them
in a specific way in order to avoid runtime crashes.
2. This was not done in the original implementation of the polkadot
connector and because of that all the tests were failing.
3. Refactoring the code so that all polkadot related dependencies are
imported dynamically fixes the issue.
4. Also fixed the issue where the getRawTransaction HTTP REST handler
was not `await` ing for the result of the connector's method which broke
the run-transaction test case (now also fixed)

Fixes hyperledger-cacti#3077

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@petermetz petermetz mentioned this issue Jul 16, 2024
Merged
5 tasks
petermetz added a commit to petermetz/cacti that referenced this issue Jul 17, 2024
@petermetz
fix(connector-polkadot): use dynamic import calls for ESM dependencies
e00cd07 
1. The polkadot libraries are all ESM-only so we have to import them
in a specific way in order to avoid runtime crashes.
2. This was not done in the original implementation of the polkadot
connector and because of that all the tests were failing.
3. Refactoring the code so that all polkadot related dependencies are
imported dynamically fixes the issue.
4. Also fixed the issue where the getRawTransaction HTTP REST handler
was not `await` ing for the result of the connector's method which broke
the run-transaction test case (now also fixed)

Fixes hyperledger-cacti#3077

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@petermetz petermetz

Labels
Flaky-Test-Automation Issues related to test stability (which is a long running issue that can never fully be solved) Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label. P3 Priority 3: Medium Polkadot Tasks related or relevant to the Polkadot connector Tests Anything related to tests be that automatic or manual, integration or unit, etc.
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging a pull request may close this issue.

fix(connector-polkadot): use dynamic import calls for ESM dependencies petermetz/cacti
3 participants
@petermetz @outSH @aldousalvarez