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.

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

Unit test assertions about errors always pass #580

Open
brentswisher opened this issue Aug 10, 2023 · 4 comments
Open

Unit test assertions about errors always pass #580

brentswisher opened this issue Aug 10, 2023 · 4 comments
Assignees

Comments

@brentswisher
Copy link
Contributor

Expected behavior
The unit tests would fail when invalid assertions are made about the errors being thrown

Actual behavior
The unit tests pass, even when making invalid assertions about the errors that are being thrown

Steps to reproduce the issue

  1. Download Pharos Locally
  2. Open the pharos icon tests file
  3. Change the "throws an error for an invalid icon name" assertion to be something silly:
  it('throws an error for an invalid icon name', async () => {
    component = await fixture(html` <test-pharos-icon name="fake"></test-pharos-icon> `).catch(
      (e) => e
    );
    expect('Could not find the moon').to.be.thrown;
  });
  1. Run tests using yarn test
  2. See that the tests still pass

Pharos version
12.21.0 - latest develop branch

Your environment

  • OS: macOS
  • Browser: N/A
  • Version: N/A

Additional information
It looks like this is just the wrong syntax when using chai, and it should be something like

expect(badFn).to.throw('salmon');

While I found this in the pharos-icon component, a similar syntax is used in 18 other places in the library and should probably all be fixed

/pharos/packages/pharos/src/components/alert/pharos-alert.test.ts
  25,47:     expect('status is a required attribute.').to.be.thrown;

/pharos/packages/pharos/src/components/button/pharos-button.test.ts
  141,82:       expect('fake is not a valid type. Valid types are: button, submit, reset').to.be.thrown;

/pharos/packages/pharos/src/components/heading/pharos-heading.test.ts
  43,46:     expect('level is a required attribute.').to.be.thrown;
  50,82:     expect('7 is not a valid heading level. Valid levels are: 1, 2, 3, 4, 5, 6').to.be.thrown;
  59,7:     ).to.be.thrown;

/pharos/packages/pharos/src/components/icon/pharos-icon.test.ts
  21,39:     expect('Could not find the moon').to.be.thrown;

/pharos/packages/pharos/src/components/image-card/pharos-image-card.test.ts
  137,81:     expect('fake is not a valid variant. Valid variants are: base, collection').to.be.thrown;
  151,7:     ).to.be.thrown;
  165,7:     ).to.be.thrown;

/pharos/packages/pharos/src/components/layout/pharos-layout.test.ts
  58,7:     ).to.be.thrown;

/pharos/packages/pharos/src/components/modal/pharos-modal.test.ts
  372,79:     expect('fake is not a valid size. Valid sizes are: small, medium, large').to.be.thrown;

/pharos/packages/pharos/src/components/text-input/pharos-text-input.test.ts
  172,7:     ).to.be.thrown;

/pharos/packages/pharos/src/components/textarea/pharos-textarea.test.ts
  96,8:       .to.be.thrown;
  114,76:     expect('blah is not a valid wrap value. Valid values are: soft, hard').to.be.thrown;

/pharos/packages/pharos/src/components/toast/pharos-toast.test.ts
  26,78:     expect('fake is not a valid status. Valid statuses are: success, error').to.be.thrown;

/pharos/packages/pharos/src/components/toggle-button-group/pharos-toggle-button.test.ts
  33,7:     ).to.be.thrown;

/pharos/packages/pharos/src/components/tooltip/pharos-tooltip.test.ts
  124,7:     ).to.be.thrown;
  181,7:     ).to.be.thrown;
@brentswisher brentswisher self-assigned this Aug 10, 2023
@brentswisher
Copy link
Contributor Author

Looked into this a little bit more, it seems like it is because fixture() is returning a promise, so chai is seeing a failed promise but not an actual error. It looks like https://github.com/domenic/chai-as-promised/ was created to extend chai and handle this but since we are using chai through @open-wc/testing' I'm not sure how to extent it like that.

I'll look into it more later, but wanted to share thoughts so far

@chrisjbrown
Copy link
Contributor

documentation on how to test open-wc component initialization
https://open-wc.org/guides/knowledge/testing/initialization/

@chrisjbrown
Copy link
Contributor

nice find Brent.
there is a closed issue in open-wc relating to chai-as-promised

open-wc/open-wc#2284

@brentswisher
Copy link
Contributor Author

@chrisjbrown That looks promising, but running into some dependency conflicts installing it, will take another look at it later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants