Skip to content

Add README.md about spec tests #4424

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

Merged
merged 4 commits into from
Feb 8, 2021
Merged

Add README.md about spec tests #4424

merged 4 commits into from
Feb 8, 2021

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Feb 5, 2021

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Feb 5, 2021

⚠️ No Changeset found

Latest commit: be560ab

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 5, 2021

Binary Size Report

Affected SDKs

  • @firebase/database-exp

    Type Base (21ee150) Head (cbe0ea5) Diff
    browser 277 kB 277 kB -59 B (-0.0%)
    esm2017 241 kB 241 kB -59 B (-0.0%)
    main 279 kB 279 kB -158 B (-0.1%)
    module 277 kB 277 kB -59 B (-0.0%)

Test Logs

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Feb 5, 2021

Size Analysis Report

Affected Products

No changes between base commit (21ee150) and head commit (cbe0ea5).

executed as a part of it.

For example, in the Web SDK (this SDK), the best way to run the spec tests is
via the Intellij IDEA IDE. After following the instructions to set up IntelliJ
Copy link
Contributor

Choose a reason for hiding this comment

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

99% of JS developers now use VS Code. The other 1% works on Firestore. I think IntelliJ is not that good of a Web IDE unless users pay for the professional edition. I would state that the tests can be run via VS Code and IntelliJ, but mostly we should point out how to use the Node test runner.

I would also give a shout out to the fact that we fake IndexedDb persistence under Node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up just removing this paragraph about IntelliJ since the instructions for running the tests are documented in each repository and don't need to be duplicated here. I also mentioned the IndexedDb persistence faking above.

```

This command assumes that this Git repository is cloned into `~/firebase-js-sdk`
and the iOS SDK is cloned into `~/firebase-ios-sdk`.
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, this is a good starting point for a spec test introduction. I think the complications arise when someone wants to go past these setup steps, but I am honestly not quite sure how we could teach someone how to write a spec test over a README. The actual tests require an insane amount of technical knowledge, and I would be wary to put all this in writing.

Even if we want to focus on the high level nature of these tests, we may still want to expand on this and write about the tags. We should mention "exclusive", "no-ios" and "no-android" (and maybe "no-web", but I don't know when that would make sense).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point about the tags. I've added information about the tags. I agree that trying to document the spec tests in their entirety is not feasible. I am mostly writing this so that the next time I need to work on the spec tests I have something to remind me of the basics, and this will hopefully be helpful to future authors as well.

Copy link
Contributor

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

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

Thanks!

@dconeybe dconeybe merged commit e8811c0 into master Feb 8, 2021
@dconeybe dconeybe deleted the dconeybe/SpecTestReadme branch February 8, 2021 16:38
@firebase firebase locked and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants