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

Technical/issue 56 organize tests #74

Merged
merged 30 commits into from
May 11, 2019

Conversation

thescientist13
Copy link
Member

@thescientist13 thescientist13 commented May 5, 2019

Related Issue

Phew 😌

Resolves #56 and

It's a start and I'm open to the naming conventions, but now every test runs in a nice isolated environment that can easily be used to replicate any particular user setup we can think of with minimal fuss to setup. 💯

Summary of Changes

  1. Renamed Setup into TestBed and refactored it to set contextualizing test running for a particular case directory
  2. Refactored test organization to be "case" based, where each case is its own standalone testing environment
  3. Created a runSmokeTest script for running common Greenwood assertions. Works good, but might need a little more work / tweaking, or we accept it's current state and hand craft the rest.
  4. Add more testing for content of default index.html and 404.html files
  5. Updated CONTRIBUTING.md with testing guidelines
  6. Removed eslint from build pipeline (we probably don't want to be linting other peoples code, we can suggest they use eslint directly like we do)
  7. Cleaned up dependencies in package.json
  8. Added a new one npm script (test:tdd) for running tests in watch mode
  9. Removed process.env.NODE_ENV from index.js
  10. Removed fixtures. (for now at least) to help promote test isolation

TODO / Help Wanted (track in a new issue for next sprint?)

  1. Fix test failure (will relax default index.html class name w/ hashing testing in smoke test), see next point
  2. Refine runSmokeTest into smaller cases for more granularity? Variations are pretty minor
    • test index.html
    • test 404.html
    • test page (takes a dynamic name with path?)
    • "gotchas" - not every build will have "hello" page, not every build will have same hash?
    • see Technical/issue 56 organize tests #74 , but adds a minor "regression"
  3. Error handling - build.config.error.*
  4. Build w/ import in a page with custom front matter
  5. Build with all custom templates (index, 404, app template, page template)
  6. Build with templates (all custom templates, custom page template)
  7. Build config w/custom workspace nested
  8. Build config w/custom workspace custom templates
  9. Should smoke include hashing, or is that an internal API? (TBD)

All test cases captured in this ticket now: #76

Thoughts / Observations

  1. I think I would like to expand on improve clarity / purpose of cli folder structure by renaming key files #57 by including the renaming of - UPDATED TICKET
    • packages/cli/tasks -> commands/
    • packages/cli/lib -> lifecycles/
    • packages/cli/lib/util -> packages/cli/lib/
  2. Smoke tests run "out of order" (at the end of all other specs)
  3. Why does build.default.workspace.nested case need a src/pages/index.md? Something to document, when users provide their own src/pages/ directory?
  4. Home page selector for default index.html are not hashed - WILL MAKE A TICKET FOR DETERMINISM

@thescientist13 thescientist13 added bug Something isn't working help wanted Extra attention is needed chore unit testing, maintenance, etc RFC Proposal and changes to workflows, architecture, APIs, etc labels May 5, 2019
@thescientist13
Copy link
Member Author

thescientist13 commented May 5, 2019

 Build Greenwood With: 
    Default Greenwood Configuration and Default Workspace w/ Nested Directories
      - should pass all smoke tests
      ✓ should create a default blog page directory
      Custom blog page directory
        ✓ should output an index.html file within the default hello page directory
        1) should have the expected heading text within the hello example page in the hello directory
        2) should have the expected paragraph text within the hello example page in the hello directory

Something must be up with the hashing, likely because the absolute path on my machine is different from CircleCI probably....

@thescientist13 thescientist13 self-assigned this May 5, 2019
@thescientist13 thescientist13 added the P0 Critical issue that should get addressed ASAP label May 5, 2019
@hutchgrant
Copy link
Member

hutchgrant commented May 8, 2019

I don't know what's wrong but yarn test:tdd script doesn't appear to be working properly. When I edit and save a test it continues just repeating:

  Build Greenwood With: 
    Empty Configuration and Default Workspace


  0 passing (5s)

constantly and that's not the test I was editing.

@thescientist13
Copy link
Member Author

Hmm, I saw that before and fixed it by upgrading mocha, but seems the issue is now back :/

Taking a look.

@thescientist13
Copy link
Member Author

thescientist13 commented May 8, 2019

Ok, saw the issue with test:tdd. Seems like if there is prebuilt output (from a previous test run) then the tests will return 0, so if you see that, try making sure all the .greenwood/ and public/ directories have been deleted first in each case directory.

I also notice that in watch mode, the tests wont clean up after themselves, (e.g. call setup.teardownTestBed)

Not sure if we should leave test:tdd and document that caveat? Still seems like a neat feature that's better than nothing, IMO. When it is working, it works pretty nice.

@thescientist13
Copy link
Member Author

Actually, a little rimraf to the rescue. 😄

Tweaked the clean task a bit, so now it will recursively clean all public/ and .greenwood/ directories

"clean": "rimraf ./**/.greenwood/** && rimraf ./**/public/** && rimraf ./coverage"

now that can be run before test:tdd to make sure there is a clean space to start from.

That should fix it now! 🔧

* integrating smoke tests

* full smoke test integration

* cleanup

* fix linting

* improve error message

* Fix test async and teardown (#77)

* fix: removed smoke test promises, implemented shared context using this

* fix: remove TODO
@thescientist13 thescientist13 merged commit d3595c4 into master May 11, 2019
@thescientist13 thescientist13 deleted the technical/issue-56-organize-tests branch May 11, 2019 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chore unit testing, maintenance, etc help wanted Extra attention is needed P0 Critical issue that should get addressed ASAP RFC Proposal and changes to workflows, architecture, APIs, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Case based / BDD organization strategy for tests
2 participants