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

Fix broken browser tests #269

Merged
merged 2 commits into from
Nov 8, 2021
Merged

Fix broken browser tests #269

merged 2 commits into from
Nov 8, 2021

Conversation

juslesan
Copy link
Contributor

@juslesan juslesan commented Nov 2, 2021

  • Replace Jest functions that do not work in the browser tests

@juslesan juslesan requested review from harbu and teogeb November 2, 2021 13:18
@github-actions github-actions bot added the network Related to Network Package label Nov 2, 2021
@teogeb
Copy link
Contributor

teogeb commented Nov 2, 2021

Would it be possible to modify the browser test setup so that it supports jest-extended?

Currently we set the standard jest object to global namespace in karma-setup.js:

window.jest = jest

We also set some additional methods there, e.g.:

jest.advanceTimersByTime = timers.advanceTimersByTime

Maybe the jest-extended methods could be injected to global namespace similarly?

@juslesan
Copy link
Contributor Author

juslesan commented Nov 4, 2021 •
edited
Loading

Would it be possible to modify the browser test setup so that it supports jest-extended?

Currently we set the standard jest object to global namespace in karma-setup.js:

window.jest = jest

We also set some additional methods there, e.g.:

jest.advanceTimersByTime = timers.advanceTimersByTime

Maybe the jest-extended methods could be injected to global namespace similarly?

I've tried to add the jest-extended functions to the browser tests but importing the matchers seems to not be compatible with browser. Simply importing the package to karma-setup.js results in weird undefined errors that seem to be caused by webpack. @teogeb

timoxley
timoxley approved these changes Nov 8, 2021
View reviewed changes
Copy link
Contributor

@timoxley timoxley left a comment

Choose a reason for hiding this comment

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

👍

teogeb
teogeb approved these changes Nov 8, 2021
View reviewed changes
Copy link
Contributor

@teogeb teogeb left a comment

Choose a reason for hiding this comment

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

Yes, good to get the tests green. Let's integrate jest-extended in another PR (or add the similar helper methods manually).

@juslesan juslesan merged commit 94f43dc into main Nov 8, 2021
@juslesan juslesan deleted the fix-network-browser-tests branch November 8, 2021 13:26
teogeb added a commit that referenced this pull request Nov 8, 2021
@teogeb
Revert "Fix broken browser tests (#269)"
a7ae7e4 
This reverts commit 94f43dc.
@teogeb teogeb mentioned this pull request Nov 8, 2021
Merged
timoxley added a commit that referenced this pull request Nov 9, 2021
@timoxley
Merge branch 'main' into NET-542-brubeck-client-browser
4c1bc51 
* main: (48 commits)
  style(client): fix eslint error
  Fix broken browser tests (#269)
  client [NET-571]: Using the Brubeck client without providing a private key (#268)
  ci: specify exact action versions for easy debugability (#266)
  ci(deps): bump actions/checkout from 2.3.5 to 2.4.0 (#271)
  docs(broker): fix mqtt example
  fix(broker): rely on default value for config.network.webrtcDisallowPrivateAddresses
  fix(broker): fix default value type in schema
  release: cli-tools 6.0.0-alpha.0
  fix: cli-tools work with alpha client
  test(network): fix describe
  Update README.md
  style(broker): break up log line
  ci(test-utils): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(protocol): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(network): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(client-dataunions): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(client-code): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(client-build): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(broker): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ...
timoxley added a commit that referenced this pull request Nov 9, 2021
@timoxley
Merge branch 'NET-542-brubeck-client-browser' into npm-workspaces
af9df43 
* NET-542-brubeck-client-browser: (50 commits)
  Revert "Revert "Merge branch 'main' into NET-542-brubeck-client-browser""
  Revert "Revert "Merge branch 'main' into NET-542-brubeck-client-browser""
  style(client): fix eslint error
  Fix broken browser tests (#269)
  client [NET-571]: Using the Brubeck client without providing a private key (#268)
  ci: specify exact action versions for easy debugability (#266)
  ci(deps): bump actions/checkout from 2.3.5 to 2.4.0 (#271)
  docs(broker): fix mqtt example
  fix(broker): rely on default value for config.network.webrtcDisallowPrivateAddresses
  fix(broker): fix default value type in schema
  release: cli-tools 6.0.0-alpha.0
  fix: cli-tools work with alpha client
  test(network): fix describe
  Update README.md
  style(broker): break up log line
  ci(test-utils): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(protocol): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(network): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(client-dataunions): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(client-code): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ...
samt1803 added a commit that referenced this pull request Nov 9, 2021
@samt1803
Merge remote-tracking branch 'origin/main' into ETH-108_registries_me…
bcce662 
…rge_main

* origin/main: (34 commits)
  broker [NET-573]: MQTT plugin improvements (#272)
  style(client): fix eslint error
  Fix broken browser tests (#269)
  client [NET-571]: Using the Brubeck client without providing a private key (#268)
  ci: specify exact action versions for easy debugability (#266)
  ci(deps): bump actions/checkout from 2.3.5 to 2.4.0 (#271)
  docs(broker): fix mqtt example
  fix(broker): rely on default value for config.network.webrtcDisallowPrivateAddresses
  fix(broker): fix default value type in schema
  release: cli-tools 6.0.0-alpha.0
  fix: cli-tools work with alpha client
  test(network): fix describe
  Update README.md
  style(broker): break up log line
  ci(test-utils): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(protocol): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(network): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(client-dataunions): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(client-code): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ci(client-build): replace styfle/cancel-workflow-action with GitHub Actions concurrency
  ...

# Conflicts:
#	packages/broker/src/broker.ts
#	packages/cli-tools/src/publish.ts
teogeb added a commit that referenced this pull request Nov 10, 2021
@teogeb
network: add support for jest-extended in browser tests (#273)
8d9f011 
Added support for jest-extended methods (e.g thetoIncludeSameMembers() matcher) in browser tests.

The injection in karma-setup.js is a bit hack as we need to import an internal file from jest-extended library:

import jestExtendedMatchers from 'jest-extended/dist/matchers'

This PR also brings back the use of jest-extended in two network test cases (reverts most of PR #269).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timoxley timoxley timoxley approved these changes

@teogeb teogeb teogeb approved these changes

@harbu harbu Awaiting requested review from harbu

Assignees
No one assigned
Labels
network Related to Network Package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@juslesan @teogeb @timoxley