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

Review automated tests #4

Open
crayola-eater opened this issue May 12, 2022 · 0 comments
Open

Review automated tests #4

crayola-eater opened this issue May 12, 2022 · 0 comments
Labels
bug Something isn't working enhancement New feature or request question Further information is requested

Comments

@crayola-eater
Copy link

  1. Think '.body' selector will need to instead be 'body'. Otherwise, document.querySelector('.body') returns null here, which is a problem in this context, as null gets passed to toHaveStyle matcher, which throws a type/validation error during the test as the matcher doesn't accept null.

https://github.com/SchoolOfCode/workshop_css-variables-b12/blob/35032a34ee5a691156054b2ec923a8c5fca49966/main.test.js#L57
https://github.com/SchoolOfCode/workshop_css-variables-b12/blob/35032a34ee5a691156054b2ec923a8c5fca49966/main.test.js#L88

All the bootcampers failed this particular test I believe for the reason above.

  1. There might be issues with using toHaveStyle in conjunction with CSS variables, where the assertion passes even when it shouldn't.

  2. Once the above has been investigated, it might be useful to break the first test up into one test per assertion, just to provide a more granular view when autograding (and so that bootcampers don't have to pass all the assertions to get 1 point). Maybe something like:

describe(LEVELS.one, () => {
  it.each(
     // Could combine all of the expected css rules for a given selector into a single string/object.
     // But doing it separately should mean we'll have a more granular view when autograding.
    [
      [".plant-header", "font-size: var(--header-size)"],
      [".plant-listing", "background-color: var(--primary-colour)"],
      [".plant-listing", "border-radius: var(--border-radius)"],
      [".plant-pic", "border-radius: var(--border-radius)"],
      ["body", "color: var(--text-colour)"],
      ["body", "background-color: var(--secondary-colour)"],
      ["button", "font-size: var(--text-size)"],
      ["button", "border-radius: var(--border-radius)"],
      ["main", "font-size: var(--text-size)"],
    ]
  )(
    "should have an element (%s) styled using CSS variables (%s)",
    (selector, cssRule) => {
      const element = document.querySelector(selector);
      expect(element).not.toBeNull();
      expect(element).toHaveStyle(cssRule);
    }
  );
});
@crayola-eater crayola-eater added bug Something isn't working enhancement New feature or request question Further information is requested labels May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant