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

(refactor): split build tests into separate files per fixture #621

Merged
merged 2 commits into from
Mar 25, 2020

Conversation

agilgur5
Copy link
Collaborator

@agilgur5 agilgur5 commented Mar 17, 2020

  • better organized that way as the tests are somewhat independent of
    one another and previously even I was confused where to put different
    types of tests

  • also refactor some test naming and some redundant usage of flags

    • especially for 0-config, where flags shouldn't be used
  • and remove testEnvironment comment as it's already set in jest config (this is why (optim/test): use node testEnvironment #405 didn't make a big difference)

(optim): now that it's one fixture per file, we only need to copy the
fixture directory once per file, which should be a speed boost

  • and sometimes may not need to re-run tsdx build when not necessary

  • also the different test files get parallelized, which is also faster

  • before: 53s avg on my machine, after: 37s avg on my machine

    • avg for running all tests
    • pretty BIG difference

Been meaning to do this for a while (as well as some other test optimizations still in-progress), and now that there's more and more fixtures (e.g. #468, #525) and more and more tests, I think now is a good time to do this

- better organized that way as the tests are somewhat independent of
  one another and previously even I was confused where to put different
  types of tests

- also refactor some test naming and some redundant usage of flags
  - especially for 0-config, where flags shouldn't be used

- and remove testEnvironment comment as it's already set in jest config

(optim): now that it's one fixture per file, we only need to copy the
fixture directory once per file, which should be a speed boost
- and sometimes may not need to re-run `tsdx build` when not necessary
- also the different test files get parallelized, which is also faster

- before: 53s avg on my machine, after: 37s avg on my machine
  - avg for running all tests
  - pretty BIG difference
- had removed some duplicate execs in the previous commit in order to
  speed up tests so that exec wouldn't need to be re-run
  - but this caused reproducibility issues, as if you change a test
    to `it.only` temporarily, it would fail hard as nothing would be
    exec'd if it weren't the first test
    - this uses an `execWithCache` function to instead naive "cache"
      the output and not re-run the same command twice-in-a-row
      - only works with sequential tests, but good enough for now
      - similar to a beforeAll() for a group of the same test, but no
        nesting or confusing variables in describe scope

- N.B. no need for stageName in cache function or something more
  complex because each stage is parallelized as multi-process
Copy link
Collaborator Author

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM!

@agilgur5 agilgur5 merged commit d234dff into jaredpalmer:master Mar 25, 2020
@agilgur5 agilgur5 mentioned this pull request Mar 26, 2020
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.

1 participant