Skip to content

fix(interop): use standaloneTransports as string instead of param#219

Merged
MarcoPolo merged 3 commits intomasterfrom
interop-sql-array-param
Jul 5, 2023
Merged

fix(interop): use standaloneTransports as string instead of param#219
MarcoPolo merged 3 commits intomasterfrom
interop-sql-array-param

Conversation

@mxinden
Copy link
Member

@mxinden mxinden commented Jul 5, 2023

When generating the test specification (docker-compose file) we enumerate all transport permutations. While some of our transports need to be upgraded (e.g. TCP and WS) some don't (e.g. QUIC or WebRTC). To differentiate the two we execute two SQL queries (queryResults and standaloneTransportsQueryResults), each with either a AND _ NOT IN or AND _ IN.

The previous implementation would use node-sqlite3 query parameters (see https://github.com/TryGhost/node-sqlite3/wiki/API#runsql--param---callback) to provide the list of transport protocols that don't need upgrading as an array.

The problem is that node-sqlite3 does not support arrays as query parameters, see TryGhost/node-sqlite3#762.

This commit uses string interpolation instead to inject the list of standalone transports into the SQL queries.

Follow-up to #210.
Fixes #218
Unblocks #217

When generating the test specification (docker-compose file) we enumerate all
transport permutations. While some of our transports need to be upgraded (e.g.
TCP and WS) some don't (e.g. QUIC or WebRTC). To differentiate the two we
execute two SQL queries (`queryResults` and `standaloneTransportsQueryResults`),
each with either a `AND _ NOT IN` or `AND _ IN`.

The previous implementation would use `node-sqlite3` query parameters (see
https://github.com/TryGhost/node-sqlite3/wiki/API#runsql--param---callback) to
provide the list of transport protocols that don't need upgrading as an array.

The problem is that `node-sqlite3` does not support arrays as query parameters,
see TryGhost/node-sqlite3#762.

This commit uses string interpolation instead to inject the list of standalone
transports into the SQL queries.
@thomaseizinger
Copy link
Contributor

Should we have an output from the action, counting the number of run tests so we can assert it after?

Copy link
Contributor

@MarcoPolo MarcoPolo left a comment

Choose a reason for hiding this comment

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

Unfortunate that we have to use string interopolation here :(

@MarcoPolo MarcoPolo enabled auto-merge (squash) July 5, 2023 16:20
@MarcoPolo
Copy link
Contributor

I'm going to merge this despite CI not being happy because the current state of master is wrong. CI is being flaky, but should be fixed by #224

@MarcoPolo MarcoPolo disabled auto-merge July 5, 2023 23:57
@MarcoPolo MarcoPolo merged commit 465065c into master Jul 5, 2023
@MarcoPolo MarcoPolo deleted the interop-sql-array-param branch July 5, 2023 23:57
codemaestro64 pushed a commit to codemaestro64/test-plans that referenced this pull request Oct 28, 2025
…bp2p#219)

* fix(interop): use standaloneTransports as string instead of param

When generating the test specification (docker-compose file) we enumerate all
transport permutations. While some of our transports need to be upgraded (e.g.
TCP and WS) some don't (e.g. QUIC or WebRTC). To differentiate the two we
execute two SQL queries (`queryResults` and `standaloneTransportsQueryResults`),
each with either a `AND _ NOT IN` or `AND _ IN`.

The previous implementation would use `node-sqlite3` query parameters (see
https://github.com/TryGhost/node-sqlite3/wiki/API#runsql--param---callback) to
provide the list of transport protocols that don't need upgrading as an array.

The problem is that `node-sqlite3` does not support arrays as query parameters,
see TryGhost/node-sqlite3#762.

This commit uses string interpolation instead to inject the list of standalone
transports into the SQL queries.

* Nit

---------

Co-authored-by: Marco Munizaga <git@marcopolo.io>
Co-authored-by: Marco Munizaga <marco@marcopolo.io>
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.

interop: stop testing webtransport with sec and mux

3 participants