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

Better documentation #19

Merged
merged 7 commits into from
Oct 18, 2024
Merged

Better documentation #19

merged 7 commits into from
Oct 18, 2024

Conversation

jonsmock
Copy link
Collaborator

@jonsmock jonsmock commented Oct 18, 2024

The goal of this PR is to improve the experience for users through documentation as well as improve code clarify for developers through docstrings comments. No major functional changes.

For documentation, the README is freshened a bit to include more installation guidance, and a new guide with additional reference documentation is created. The root README as well as the guide/reference docs are rendered by Docsify here: https://viasat.github.io/dctest/ (currently set to this branch in GitHub Pages). The root README also serves as our docsify docs home (i.e. relative path / in the browser), but to accomplish this, we have to configure docsify to point to it by URL. In other words, the GitHub Pages link above is currently serving the main branch's README but all other docs from the better-documentation branch. Once this PR is merged, I will repoint GitHub Pages, so that all config and docs are pointed at main. To see the new README changes, see the default GitHub rendered version on the branch: https://github.com/Viasat/dctest/tree/better-documentation .

The main weakness of the reference docs are that they are disconnected from the JSON schema and implementation (list of expressions functions/methods). I hope to recover the linkage/connection here somehow in the future.

As additional documentation for early adopter users, I've added a CHANGELOG to summarize our past releases to set the pattern for future additions or deprecations, if need be. We have enough internal users that this will help migrate people through to an eventual 1.0 release.

For developers I've incorporate the helpful feedback from PR 15 by adding docstrings and code comments, attempting to clarify the purpose, inputs/outputs, assumptions, and in some cases the implementation of the code itself. In some cases the prose is longer than perhaps expected, which might indicate we have some design/refactoring work in that area, but my goal here was just to document what exists as clearly as I could.

Misc: add a couple npm scripts to our package.json to create one-liners for running all tests (unit and integration) and serving the docs locally with docsify.

Attempt to bring additional clarity to the code by adding docstrings and
comments, making some of the implicit inputs, outputs, assumptions, and
intentions more explicit. Often these are documented in previous commit
messages but not evident in the code itself.

In some cases, adding docstrings or comments may reveal a function is
not yet factored as well as it could be, but the primary goal of this
commit is to mainly document what is there. Any descriptions that feel
excessively long or complicated might can be candidates for future
simplification or refactoring.
Start a CHANGELOG to facilitate early adopters' upgrades, while we are
still pre-1.0.
Demonstrate installation via checkout, npm, or Docker Hub.

Also clarify a common question regarding the first argument (project
name) for the dctest CLI invocation.

Misc: add language annotations on code snippets.
Starts a simple prose guide on writing a first test, which references
the other runnable examples as well as new reference documentation.

The reference documentation mirrors the JSON schemas (input and
results-file) and implementation (expressions), but it is unfortunately
not _tied_ to it or linted by it in any way currently. This is an area
for future improvement, but the first goal is getting easy to use, human
readable documentation up.
@jonsmock jonsmock requested review from kanaka and gdw2 October 18, 2024 18:24
Docsify allows us to simply commit and push markdown files to create
nicely formatted and navigable documentation with no build step! The
markdown documentation and a short index.html can be served via
GitHub Pages. Docsify fetchs the markdown file via XMLHttpRequest and
renders it as HTML on the fly.

Our docsify configuration points the top level landing page (/) at
the project's root README.md on GitHub to avoid duplication. If
rendering chagnes to the README.md landing page locally, index.html
needs to be temporarily adjusted to use a symlink to the local
README.md (as indicated by the comment).

Additional docsify configuration and plugins are used to use a
non-standard theme and add language syntax highlighting, which is done
on a per-language basis and uses the language annotations from code
fences.

Add docsify dev dependency and npm script for local rendering.
Copy link
Collaborator

@kanaka kanaka left a comment

Choose a reason for hiding this comment

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

Looks great!

;; (P/let [sts x
;; sts (f' sts)
;; sts (g' sts)]
;; (h' sts))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the ticks in the conceptual transformation?

Copy link
Collaborator Author

@jonsmock jonsmock Oct 18, 2024

Choose a reason for hiding this comment

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

f/g/h are the original forms, and we are transforming them substantially by wrapping in try/catch and pending? checks (in addition to the normal -> transformation). It felt like a lie to just write f/g/h, so the prime was attempting to indicate that transformation ... maybe it confuses more than it helps?

* `tests` - all tests executed as part of dctest run
* `errors` - an array of errors encountered during execution

> NOTE: The results file is formally defined with [JSON Schema](https://github.com/Viasat/dctest/blob/main/schemas/results-file.yaml).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not something to change in this PR, but how far is this form from a subset of the CTRF standard?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Short version: not a very far! We would add a few required things that we don't use and under-specify some things that users might want to depend on, but the delta is pretty small.

Detailed List:

  • all of our top-level keys move under a new top-level "results" key
  • add a required "tools" key to describe the version of dctest itself (a good idea)
  • add some required counts, set to zero, under the "summary" key (ex: "pending" and "other")
  • move our "errors" list under a key called "extra", an unspecified "catch all" in CRTF
  • add required "duration" key to tests
  • consider adding a lot of keys to test that CTRF has as optional
  • move some of the step keys that we currently return into the unspecified "extra" catch-all, because CTRF's steps are defined more narrowly

As a practical example, "start" and "stop" are specified by CTRF but optional. If CTRF is our only reference spec for results, users will have a hard time knowing we are committed to returning those vs. other keys like "browser" or "screenshot" that perhaps will never make sense for us to support. Similarly, the format of returned errors goes into the "extra" bucket often, and I think users probably will want to depend on this to bubble up error messages into their GHA summaries and what not (I could be wrong).

If we want to go the CTRF route after all (which I would still be fine with - no strong opinion), we could indicate we conform to CTRF and then consider having a secondary schema that specifies the additional pieces we commit to. I like the idea of something like CTRF becoming a sort of standard. Currently, because it happened later, it has become the target for converting other testing tools' outputs (mocha, JUnit, etc), when a common standard is important, but I haven't seen tools that output it straight-away (chicken-egg heh; also I could be missing them).

@jonsmock jonsmock merged commit 6b904fd into main Oct 18, 2024
9 checks passed
@jonsmock jonsmock deleted the better-documentation branch October 18, 2024 22:02
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.

2 participants