Summary:
This import does not need to be dynamic; the fact that it is loses us
safety for no benefit. (When I originally wrote it, it was less
obviously bad, but the surrounding code has changed over time.)
Test Plan:
Running `yarn flow` suffices, and now actually checks this module
instead of typing it as `any`. Running `yarn test --full` is nice, too.
wchargin-branch: webpack-remove-dynamic-import
Summary:
We want to remove the repository selector dropdown on the cred explorer
homepage and instead render a separate web page for each project. To do
this, we need to know which pages to render statically. We choose to
ingest this information from the state of the repository registry at
build time.
This commit adds an environment variable `REPO_REGISTRY` whose contents
are the stringified version of the repository registry, or `null` if
SourceCred has been built for the backend. This variable is defined with
Webpack’s `DefinePlugin`, so any code bundled by Webpack can refer to it
via `process.env.REPO_REGISTRY` both on the server and in the browser.
Paired with @wchargin.
Test Plan:
Sharness tests modified; running `yarn test --full` suffices.
Summary:
Prior to this commit, running `yarn test full` (instead of `--full`)
would silently behave the same way as `yarn test`. This is misleading.
It now fails.
Test Plan:
Run `! yarn test full && yarn test --full`.
wchargin-branch: test-validate-arguments
Currently version is located in `homepage/`, which doesn't make much
sense, since it's versioning the whole project.
We move it to core.
Test plan: `yarn test --full`
Summary:
This was used for ad hoc testing of the Mirror module before it was
integrated into SourceCred. We haven’t kept it up to date with schema
changes, and it is no longer needed: you can just run `sourcecred load`.
This was also the only untested code in the `graphql/` package, so it is
nice to remove it.
Test Plan:
Running `yarn test --full` passes.
wchargin-branch: remove-mirror-demo
The referenced pull request mistakenly didn't update `config/paths.js`,
which caused build_static_site to fail in `yarn test full`.
Test plan: `yarn test full` now passes.
Summary:
Fun facts:
- `new Date().getDay()` does not return the day of the month. It
returns the day of the _week_ as an integer `0 ≤ n ≤ 6`.
- `new Date().getDate()` returns the day of the month from 1 to 31.
- `new Date().getMonth()` really does return the month, but _this_ one
is zero-based!
All this to say, my implementation in #901 was a bit flawed.
Why didn’t I notice at the time? I wrote and tested the change on
2018-10-01, which was a Monday, so both `getDay()` and `getDate()` were
in fact `1`. As for me failing to notice that `getMonth()` was off by
one—well, sometimes I’m very dumb.
Test Plan:
```shell
$ NODE_ENV=development node -e '
> require("./config/env");
> console.log(process.env.SOURCECRED_GIT_STATE);
> '
{"commitHash":"f9bb75ef71c5","commitTimestamp":"20181030-1518","dirty":true}
$ date -I
2018-10-30
```
wchargin-branch: env-fix-date-formatting
Summary:
As of this commit, `node ./bin/sourcecred.js load` uses the Mirror code,
and the legacy continuation-fetching code is not included in the
`sourcecred.js` bundle.
We do not yet perform the commit prefetching described in #923. The code
should be plenty fast for repositories that merge pull requests at least
occasionally.
Test Plan:
Running `yarn test --full` passes. Loading `sourcecred/sourcecred` works
and generates a reasonable credit attribution. Loading it again
completes immediately.
wchargin-branch: fetchGithubRepo-mirror
Summary:
This makes significant progress toward #923. As of this commit, it is
possible to use the Mirror module for the whole loading pipeline. This
process may be slow for repositories that do not use pull requests at
all (more precisely, that have large connected commit subgraphs none of
whose nodes is the merge commit of a pull request; see #920 for details)
so it is not yet the default codepath.
Test Plan:
Existing unit tests should suffice. For extra testing, I’ve added a
script that fetches a repository both via the old continuations logic
and the new Mirror logic, then constructs relational views and checks
whether the data is the same. For `example-github`, the views are
identical. For `sourcecred`, they are not: the old continuations logic
erroneously omits two commits, which the Mirror logic includes.
You can run the test like this:
```
$ node ./bin/testContinuations.js \
> sourcecred sourcecred MDEwOlJlcG9zaXRvcnkxMjAxNDU1NzA= \
> /tmp/continuations.json /tmp/mirror.json \
> 2> >(jq . >&2)
{
"child": "0d38dde23a6de831315f3643a7d2bc15e8df7678",
"parent": "cb8ba0eaa1abc1f921e7165bb19e29b40723ce65",
"type": "UNKNOWN_PARENT_OID"
}
{
"child": "d152f48ce4c2ed1d046bf6ed4f139e7e393ea660",
"parent": "de7a8723963d9cd0437ef34f5942a071b850c0e7",
"type": "UNKNOWN_PARENT_OID"
}
Different. Saving to disk...
```
Use `diff -u <(jq . /tmp/continuations.json) <(jq . /tmp/mirror.json)`
to inspect the differences, and note that exactly the two missing
commits have been added and that there are no other changes. (The diff
is small: just 51 lines of nicely formatted JSON.) The full log is here:
<https://gist.github.com/wchargin/e159cac9dcf3cc3b1efbd54f59e24e0b>
I also generated the `sourcecred/sourcecred` cred attribution and viewed
it with `yarn start`, which seems to work fine.
wchargin-branch: relationalview-new-data-format
Summary:
The included schema is forked from the one in `graphql/demo.js`.
Primitive types have been added, and the `parents` connection has been
added to commit objects per #920. (We do not include this in the demo
script because without prefetching it would take a long time to load.)
Test Plan:
Unit tests added; run `yarn unit`. Then run `yarn backend` and verify
that `node ./bin/generateGithubGraphqlFlowTypes.js` generates exactly
the same output as in the types file:
```
$ node ./bin/generateGithubGraphqlFlowTypes.js |
> diff -u - ./src/plugins/github/graphqlTypes.js
$ echo $?
0
```
Change the `graphqlTypes.js` file and verify that `yarn unit` fails.
As the build config has been changed, a `yarn test --full` is warranted.
It passes.
Finally, I have manually verified that the schema is consistent with the
documentation at <https://developer.github.com/v4/object/repository/>
and related pages.
wchargin-branch: github-schema-flow-types
Summary:
Closes#902. CircleCI continues to work fast and well, on both the
commit and nightly workflows. Travis continues to be slower. It is time.
I have disabled Travis for `sourcecred/sourcecred` via:
<https://travis-ci.org/profile/sourcecred>
This commit removes the vestigial configuration files and code.
Test Plan:
Running `yarn test` still works. Running `yarn test --full` still works,
and properly invokes `sharness-full` and the various extra tests.
Running `git grep -i travis` yields no results.
wchargin-branch: ci-remove-travis
Summary:
This completes the minimum viable public API for the `Mirror` class. As
described on the docstring, the typical workflow for a client is:
- Invoke the constructor to acquire a `Mirror` instance.
- Invoke `registerObject` to register a root object of interest.
- Invoke `update` to update all transitive dependencies.
- Invoke `extract` to retrieve the data in structured form.
It is the third step that is added in this commit.
In this commit, we also include a command-line demo of the Mirror
module, which exercises exactly the workflow above with a hard-coded
GitHub schema. This can be used to test the module’s behavior with
real-world data. I plan to remove this entry point once we integrate
this module into SourceCred.
This commit makes progress toward #622.
Test Plan:
Unit tests included for the core functionality. The imperative shell
does not have automated tests. You can test it as follows.
First, run `yarn backend` to build `bin/mirror.js`. Then, run:
```shell
$ node bin/mirror.js /tmp/mirror-demo.db \
> Repository MDEwOlJlcG9zaXRvcnkxMjMyNTUwMDY= \
> 60
```
Here, the big base64 string is the ID for the sourcecred/example-github
repository. (This is listed in `graphql/demo.js`, alongside the ID for
the SourceCred repository itself.) The value 60 is a TTL in seconds. The
database filename is arbitrary.
This will print out a ton of output to stderr (all intermediate queries
and responses, for debugging purposes), and then the full contents of
the example repository to stdout.
Run the command again, and it should finish instantly. On my machine,
the main function runs faster than the Node startup time (50ms vs 60ms).
Then, re-run the command, changing the TTL from `60` to `1`. Note that
it sends off some queries and then prints the same repository.
It is safe to kill the program at any time, either while waiting for a
response from GitHub or while processing the results, because the mirror
module takes advantage of SQLite’s transaction-safety. Intermediate
updates will be persisted, so you’ll lose just a few seconds of
progress.
You can also of course dive into the generated database yourself to
explore the data. It’s good fun.
wchargin-branch: mirror-e2e-update
Summary:
Git only learned `--date=format:...` in Git 2.6.0. Some old Docker
images have older versions of Git. It’s not too much work to reimplement
this particular bit of functionality, so this commit does so.
Test Plan:
Install Git 2.1.4 (as used on CircleCI): from the Git repository for
Git itself, run:
git checkout v2.1.4
make
make install
Watch `yarn test --full` pass. Before this commit, `yarn unit` failed.
wchargin-branch: env-support-git-2.1.4
Summary:
Our environment-stripping logic used in `config/env.js` to read the
current Git state included stripping the `PATH` environment variable.
This had the effect that the system Git executable would always be used
in preference to a user-installed version.
Test Plan:
Run `/usr/bin/git --version`, and then install a different version of
Git. (For instance, check out an old tag, then `make && make install`
from the `git/git` repository.) Then, add
```js
console.log(execFileSync("git", ["--version"], {env}).toString());
```
to `getGitState` in `config/env.js`, and run
NODE_ENV=development node ./config/env.js
Note that this prints the version of the system Git before this change,
and the user Git after this change.
Alternately, local-install a version of Git earlier than 2.6.0, and note
that `yarn unit` now _fails_ because the `--date=format:…` syntax is not
known to such versions of Git. Prior to this commit, the tests would
pass as long as the system Git were more recent.
wchargin-branch: env-git-path
This modifies the behavior when loading the Git plugin so that it
serializes the Repository as well as the graph. This will allow us to
get extra information, like the commit headline, to the Git plugin in
the frontend.
As an added bonus, we can now refactor `loadRepositoryTest` to depend on
`sourcecred.js load` rather than `loadAndPrintRepository`. As this was
the only use for `loadAndPrintRepository`, we can safely delete it. This
improves our test quality because it means we are also testing the
actual CLI behavior.
Note that the switch from using `stringify` to `json.tool` for
pretty-printing has resulted in a trivial diff in the snapshot.
Test plan: `yarn test --full` passes.
According to the [jest docs], setting `maxWorkers=4` can substantially
improve performance on CI.
This commit sets `maxWorkers=4` when running `yarn unit` as a part of
`yarn test`. Based on my local testing (see data below), this improves
performance locally in addition to the expected performance improvement
on travis.
\## Testing `yarn unit` by itself
```
yarn unit --maxWorkers=1
15.28s user 3.71s system 112% cpu 16.848 total
15.13s user 3.68s system 112% cpu 16.708 total
15.32s user 3.76s system 112% cpu 16.917 total
15.91s user 3.74s system 112% cpu 17.425 total
15.61s user 3.76s system 113% cpu 17.125 total
yarn unit --maxWorkers=2
19.43s user 4.03s system 212% cpu 11.061 total
19.86s user 4.19s system 210% cpu 11.407 total
21.19s user 4.26s system 213% cpu 11.902 total
20.68s user 4.51s system 212% cpu 11.873 total
20.78s user 4.26s system 212% cpu 11.780 total
yarn unit --maxWorkers=4
29.43s user 5.14s system 389% cpu 8.865 total
29.99s user 5.18s system 392% cpu 8.961 total
32.22s user 5.52s system 390% cpu 9.659 total
33.99s user 5.97s system 395% cpu 10.097 total
33.38s user 5.93s system 395% cpu 9.933 total
yarn unit --maxWorkers=8
48.21s user 6.57s system 621% cpu 8.815 total
51.61s user 7.16s system 610% cpu 9.622 total
59.48s user 7.82s system 621% cpu 10.833 total
58.18s user 8.10s system 624% cpu 10.607 total
58.92s user 8.22s system 620% cpu 10.817 total
unset
46.27s user 6.44s system 599% cpu 8.799 total
49.08s user 7.04s system 600% cpu 9.342 total
54.85s user 7.52s system 600% cpu 10.383 total
55.66s user 7.52s system 605% cpu 10.438 total
53.77s user 7.50s system 604% cpu 10.142 total
```
\## Testing `yarn test`
```
maxWorkers=1
46.65s user 5.92s system 249% cpu 21.038 total
47.94s user 5.81s system 251% cpu 21.354 total
51.50s user 6.44s system 260% cpu 22.234 total
52.60s user 6.65s system 268% cpu 22.077 total
53.04s user 6.27s system 266% cpu 22.278 total
maxWorkers=2
56.13s user 6.13s system 409% cpu 15.204 total
63.32s user 7.22s system 412% cpu 17.091 total
64.82s user 7.19s system 422% cpu 17.027 total
64.59s user 7.41s system 417% cpu 17.227 total
65.40s user 7.30s system 419% cpu 17.318 total
maxWorkers=4
74.64s user 7.60s system 584% cpu 14.066 total
82.69s user 8.43s system 582% cpu 15.643 total
85.00s user 8.68s system 591% cpu 15.835 total
84.81s user 8.58s system 595% cpu 15.690 total
85.22s user 8.59s system 596% cpu 15.719 total
maxWorkers=4 and everything depends on unit
59.29s user 6.01s system 378% cpu 17.261 total
62.99s user 6.64s system 375% cpu 18.564 total
65.54s user 7.31s system 375% cpu 19.419 total
63.24s user 7.13s system 379% cpu 18.548 total
63.68s user 7.13s system 383% cpu 18.457 total
maxWorkers=8
92.85s user 8.13s system 643% cpu 15.702 total
101.63s user 9.21s system 632% cpu 17.510 total
101.63s user 9.23s system 636% cpu 17.428 total
101.81s user 9.32s system 633% cpu 17.546 total
101.62s user 9.39s system 632% cpu 17.542 total
unset
88.75s user 8.15s system 646% cpu 14.988 total
96.43s user 9.23s system 631% cpu 16.739 total
98.27s user 9.17s system 638% cpu 16.819 total
98.46s user 9.01s system 642% cpu 16.729 total
98.53s user 9.15s system 637% cpu 16.889 total
unset + everything depends on unit
76.02s user 7.61s system 486% cpu 17.208 total
79.14s user 8.26s system 484% cpu 18.030 total
84.32s user 9.19s system 488% cpu 19.136 total
84.92s user 9.14s system 497% cpu 18.919 total
84.46s user 8.94s system 492% cpu 18.965 total
```
Test plan: `yarn test` passes here and on travis
[jest docs]: https://jestjs.io/docs/en/troubleshooting
Summary:
CI mode prevents Jest from automatically writing snapshots, and also
causes obsolete snapshots to be an error instead of a warning. This is
consistent with the behavior on Travis, where the `CI=1` environment
variable is set. It should thus be the default when running `yarn test`
(but not `yarn unit`).
Test Plan:
Add a file `src/foo.test.js`:
```js
// @flow
describe("foo", () => {
it("bar", () => {
expect("baz").toMatchSnapshot();
});
});
```
Note that `yarn test` fails with the message, “new snapshot was not
written”.
Revert this change, then re-run `yarn test`; note that it passes,
writing a snapshot.
Then, reinstate this change and delete `src/foo.test.js`. Note that
running `yarn test` fails, due to an obsolete snapshot. Revert the
change again, and watch `yarn test` pass despite the obsolete snapshot.
Finally, remove the snapshot. :-)
wchargin-branch: test-jest-ci
Summary:
Lots of tests need the output of `yarn backend`. Before this commit,
they tended to create it themselves. This was slow and wasteful, and
also could in principle have race conditions (though in practice usually
tended not to).
This commit updates tests to respect a `SOURCECRED_BIN` environment
variable indicating the path to an existing directory of backend
applications.
Closes#765.
Test Plan:
Running `yarn test --full` passes.
Prepending `echo run >>/tmp/log &&` to the `backend` script in
`package.json` and running `yarn test --full` results in a log file
containing only one line, indicating that the script really is run only
once.
wchargin-branch: deduplicate-backend
Summary:
This commit removes the `config/backend.js` script and replaces it with
a direct invocation of Webpack. This enables us to use command-line
arguments to Webpack, like `--output-path`.
Test Plan:
Note that `rm -rf bin; yarn backend` still works, and that the resulting
applications work (`node bin/sourcecred.js load`). Note that `yarn test`
and `yarn test --full` still work.
wchargin-branch: backend-webpack-direct
Summary:
We currently configure the Babel config with environment variables: in
particular, the `SOURCECRED_BACKEND` environment variable causes Babel
to target Node instead of the browser. The relevant lines are copied
from `scripts/backend.js`.
The environment variable mechanism is slightly clunky, especially as it
requires the Webpack config module to be impure, but it works okay for
our purposes. We could adopt a more principled solution—setting the
`options` argument to the Babel loader in the backend Webpack config—but
this would require redesigning the Babel config system, which would take
a moderate amount of effort.
Test Plan:
As of this commit, `yarn backend` has bitwise identical output to
directly invoking Webpack:
```shell
$ yarn backend >/dev/null 2>/dev/null
$ shasum -a 256 bin/* | shasum -a 256
c4f7494c3ba70e5488ff4a8b44550e478a2a8b27fa96f286123f9566fd28f7be -
$ NODE_ENV=development node ./node_modules/.bin/webpack \
> --config ./config/webpack.config.backend.js >/dev/null 2>/dev/null
$ shasum -a 256 bin/* | shasum -a 256
c4f7494c3ba70e5488ff4a8b44550e478a2a8b27fa96f286123f9566fd28f7be -
```
wchargin-branch: backend-set-babel-flags
Summary:
Over the past few commits, I have accidentally removed all Flow errors
from the Webpack config. We can now use Flow on that file to prevent any
new errors from creeping in.
Test Plan:
Running `yarn flow` suffices.
wchargin-branch: backend-flow
Summary:
Previously, our `webpack.config.backend.js` file actually exported a
function that could be used to make a Webpack configuration object.
(This is not to be confused with the late `makeWebpackConfig.js`, which
actually exported a configuration object!)
In addition to being confusing nomenclature, this was a sneaky trap for
CLI users. Invoking `webpack --config config/webpack.config.backend.js`
would actually work, but do the wrong thing: Webpack _allows_ your
configuration object to be a function, but with different semantics. In
particular, the result was that Webpack would emit the build output into
your current directory instead of into `bin/`.
This commit fixes that by making `webpack.config.backend.js` export the
Webpack configuration object for the backend JavaScript applications.
The logic to change the path is now handled by the caller, by
overwriting `config.output.path`; this is exactly [the same approach
that the Webpack CLI takes when given an `--output-path`][1], so it’s
okay with me.
[1]: 368e2640e6/bin/convert-argv.js (L406-L409)
Test Plan:
Run `yarn backend` and `yarn backend --dry-run`. Note that each runs
with appropriate output (both emitted files and console logs).
wchargin-branch: backend-webpack-config-object
Summary:
We lint separately, with `yarn lint`. There’s no need to duplicate this
effort.
Test Plan:
Introduce a lint error, for instance by adding `("unused expression");`
to `src/cli/main.js` and `src/app/App.js`. Note that `yarn lint` fails
but `yarn backend` and `yarn start` and `yarn build` succeed.
wchargin-branch: webpack-no-lint
Summary:
Both the backend and the web builds want to empty the build directory
before starting. This commit makes them use the same codepath, reducing
the amount of work that `scripts/backend.js` does so that we can more
easily remove it (#765).
Test Plan:
```shell
$ touch ./bin/wat
$ yarn backend >/dev/null 2>/dev/null
$ file ./bin/wat
./bin/wat: cannot open `./bin/wat' (No such file or directory)
```
wchargin-branch: backend-empty-backend-build-directory
Summary:
This simplifies interfaces everywhere.
See also #216, which did the opposite of this as a temporary fix due to
a Babel/Webpack interaction that no longer exists as of #766.
Test Plan:
Note that `node bin/sourcecred.js load sourcecred/example-git` still
works (after `yarn backend`). Note that `yarn test` still works. These
demonstrate that the module works from both a Webpack context and a Node
context. Note that `git grep --name-only execDependencyGraph` yields
exactly those files touched in this commit. Note that `yarn test --full`
passes.
wchargin-branch: commonjs-execDependencyGraph
Summary:
The approach used by `create-react-app` (the source of this code) seems
to be the following: write everything as ES6 modules (`import`/`export`)
and bundle everything with Webpack—in particular, do not transform
modules with Babel.
This works fine until you want to also access some of these modules from
raw Node. Node only supports CommonJS modules, so these “polyglot”
modules must be CommonJS-compatible. But CommonJS modules are not nicely
compatible with ES6 modules: you need to set `exports.default` instead
of `module.exports` in the provider, and dereference `.default` after
each `require`. Babel will perform these transformations for us if we
ask it to. In this patch, we do so.
Test Plan:
Note that `yarn test --full` passes and `yarn start` still works.
(Follow-up commits will exercise this functionality.)
wchargin-branch: transform-es6-modules
Summary:
The distinction was useful while `makeWebpackConfig` was being developed
(between #562 and #570), but is now confusing: we have a web config and
a backend config, and it is clearer if we name them as such.
Test Plan:
All of `yarn start`, `yarn build`, and `yarn test --full` work.
wchargin-branch: webpack-config-web
Test Plan:
Run `mkdir /tmp/out; cd /tmp/out; python -m SimpleHTTPServer`. In
another shell, run `./scripts/build_static_site.sh --target /tmp/out`.
Then, `curl localhost:8000`. Before this commit, this would have yielded
an `OSError` because the cwd of the Python process had been removed.
As of this commit, it works fine.
Also, run `git grep -c rimraf` and note only `yarn.lock:15`.
wchargin-branch: webpack-empty-build-directory
Summary:
This commit changes the CLI to use the code in `cli` instead of `oclif`.
A subsequent commit will remove the dependency on OClif altogether.
Resolves#580.
Test Plan:
Note that `yarn backend; node bin/sourcecred.js help` works. Note that
the documentation in the README is still correct.
wchargin-branch: cli-replace-oclif
Summary:
This commit includes a minimal usage of an actual CLI application. It
provides the `help` command and no actual functionality.
Test Plan:
Unit tests added, with full coverage. To see it in action, first run
`yarn backend`, then run `node bin/cli.js help`.
wchargin-branch: cli-beginnings
Summary:
Per #580, we aim to remove OClif. To do so, we move the old system to a
directory `oclif`, and will create the new system in the now-vacant
`cli` directory.
Test Plan:
Note that `yarn backend` still builds, that `node bin/sourcecred.js`
still has `help` and `load`, and that `git grep -wc cli` yields only
`yarn.lock:9`.
wchargin-branch: rename-cli-to-oclif
Summary:
This is a follow-up to #746, wherein we exposed our fixed `env` to the
backend applications. We now extend that environment so that it can also
access the user’s runtime environment—i.e., the native values of
`process.env`.
(This is in contrast to the frontend bundles `main.js` and especially
`ssr.js`, where this is not and should not be the case: the environment
must be fixed at build time.)
Test Plan:
Add to the top of `async run()` in `src/cli/commands/load.js`:
```js
console.log(require("../../app/version").VERSION_SHORT);
console.log(process.env.AT_RUNTIME);
```
Run `yarn backend` and `AT_RUNTIME=wat node bin/sourcecred.js load`.
Ensure that the version number and the string `wat` are both printed.
(Before this patch, the string `undefined` would be printed instead of
`wat`.)
wchargin-branch: backend-extensible-env
Test Plan:
Add `console.log(require("../../app/version").VERSION_SHORT);` to the
top of `async run()` in `src/cli/commands/load.js`. Run `yarn backend`
and `node bin/sourcecred.js load`, and note that it prints the current
version number. Before this change, it would have raised an error:
```
Error: gitState: not a string: undefined
at parseGitState (~/git/sourcecred/bin/commands/load.js:1160:64)
```
because the requisite environment variables were not included.
Also, `yarn test --full` passes.
wchargin-branch: backend-env
Summary:
We can now set, at build time, a URL to be displayed at the top of the
prototype, encouraging users to provide feedback. If the URL is not
provided, it defaults to the appropriate topic on the SourceCred
Discourse instance.
The result looks like this:
![Screenshot of the feedback URL in the prototype][screenshot]
[screenshot]: https://user-images.githubusercontent.com/4317806/44814824-a238b380-ab92-11e8-88c8-dfbae27ca496.png
Test Plan:
Unit tests added to `yarn sharness-full` and `yarn unit`.
You can run `yarn start` to see the message with the default URL, or
`SOURCECRED_FEEDBACK_URL=http://example.com/ yarn start` to specify a
custom URL.
wchargin-branch: feedback-url
Summary:
The version number displayed in the application now displays much more
specific information. It now lists the Git commit from which the build
was constructed, and will identify whether we have accidentally deployed
a development instance (which would be slow) or an instance with
uncommitted changes (which would be bad).
The version information is computed during the initialization of the
Webpack config. For development, this means that it is computed when you
run `yarn start`, and not updated thenafter. If the stale information
presents actual confusion, we would need to backport Webpack 4’s support
for runtime values in `DefinePlugin` to Webpack 3 (or upgrade Webpack
by a major version).
Test Plan:
The logic for `GitState` and `Environment` has existing tests. With both
a clean tree and a dirty tree, run `yarn start` and build the static
site, and check that the resulting versions are correct.
wchargin-branch: use-rich-version-types
Summary:
We don’t use or want these. Injecting an arbitrary family of variables
from the client’s host environment seems like a Bad Idea.
Test Plan:
The usual `yarn start`, static site, and `yarn test --full` still work.
wchargin-branch: remove-reactapp-vars
Summary:
Cargo-culting `reduce` doesn’t make something “functional” or “good”;
forcing a `for`-loop into a `reduce` with impure callback is abhorrent.
Test Plan:
Into `config/stopship.js`, write:
```js
console.log(JSON.stringify(require("./env")()));
```
Then run `NODE_ENV=test node config/stopship.js` before and after this
commit and note that the output is identical.
wchargin-branch: simplify-getClientEnvironment
Summary:
Now that the main functionality of #643 has been implemented, we no
longer have any use for the “public URL” property. In fact, its presence
is actively harmful, as it suggests that the gateway may be known before
runtime, which is confusing and false.
Closes#643.
Test Plan:
Running `yarn start` works. Building the static site works.
Invoking `git grep -i 'public.\?url'` finds no matches.
Also, `yarn test --full` passes.
wchargin-branch: remove-public-url
Summary:
This is the first observable step toward #643. Assets whose paths are
known as literals at server-side rendering time are now referenced via
relative paths. This means that the favicon and JavaScript bundle can be
loaded from an arbitrary gateway. The actual bundle code will still only
work when loaded from `/`.
This commit stands alone so that the enclosing change to the Webpack
config can be in as small a change as possible.
Test Plan:
- Note that `yarn start` still works.
- Run `./scripts/build_static_site.sh` to build the site into, say,
`/tmp/gateway`.
- Run a static web server from `/tmp/gateway/` and note that (a) the
paths listed in the page source are relative, and (b) everything
works as intended, with no console messages in either Chrome or
Firefox.
- Run a static web server from `/tmp/` and navigate to `/gateway/` in
the browser. Note that the favicon and JavaScript are correctly
noted, but that the router raises an error because it is trying to
load a non-existent route. (This behavior is unchanged.)
wchargin-branch: relative-lexically-static
Summary:
In addition to the obvious benefit of having a favicon, this gets rid of
a 404 Not Found error on our home page, tremendously boosting our hacker
cred.
Test Plan:
The favicon is displayed in both `yarn start` and the static site (as a
result of the build script). The added build test fails before this
change.
wchargin-branch: add-favicon
Summary:
This is a follow-up to #514, wherein we disabled new service workers and
instructed any existing service workers to self-destruct. (See that PR
for the rationale.) This commit removes them from our codebase entirely,
enabling us to slim down our build process and our build output.
Test Plan:
Running `yarn start` still works. Building the static site and exploring
it works, too.
wchargin-branch: remove-sw
Summary:
We plan to allow plugins to store permanent data in `$SC/data/` and
temporary, ephemeral, or intermediate data in `$SC/cache/`. The latter
subtree will be excluded from the static site at build time, so it
behooves us to also exclude it from the development environment.
Test Plan:
Run `yarn start`. Then,
```shell
$ root='localhost:8080/api/v1/data'
$ curl -sI "${root}/repositoryRegistry.json" | head -1
HTTP/1.1 200 OK
$ curl -sI "${root}/data/sourcecred/example-git/github/view.json" | head -1
HTTP/1.1 200 OK
$ curl -sI "${root}/cache" | head -1
HTTP/1.1 400 Bad Request
$ curl -sI "${root}/cache/" | head -1
HTTP/1.1 400 Bad Request
$ curl -sI "${root}/cache/foo" | head -1
HTTP/1.1 400 Bad Request
$ curl -sI "${root}/cache/foo/bar/baz" | head -1
HTTP/1.1 400 Bad Request
```
Also, check that the app still works.
wchargin-branch: exclude-cache-from-dev-server
Summary:
We never use the `node ./bin/sourcecred.js start` command. This command
contains an Express server to combine the static files with the build
output, which duplicates the logic in our Webpack config, which we
actually use (with `yarn start`). Once we actually want the command line
entry point to be a useful tool for end users, we can consider
reimplementing it the right way, whatever that may be. Until then, it’s
simply one more thing to keep in sync.
Test Plan:
Running `yarn test --full` passes; the `load` CLI command still works;
running `yarn start` still works.
wchargin-branch: remove-start
Summary:
We will shortly want to perform testing of shell scripts; it makes the
most sense to do so via the shell. We could roll our own testing
framework, but it makes more sense to use an existing one. By choosing
Sharness, we’re in good company: `go-ipfs` and `go-multihash` use it as
well, and it’s derived from Git’s testing library. I like it a lot.
For now, we need a dummy test file; our test runner will fail if there
are no tests to run. As soon as we have a real test, we can remove this.
This commit was generated by following the “per-project installation”
instructions at https://github.com/chriscool/sharness, and by
additionally including that repository’s `COPYING` file as
`SHARNESS_LICENSE`, with a header prepended. I considered instead adding
Sharness as a submodule, which is supported and has clear advantages
(e.g., you can update the thing), but opted to avoid the complexity of
submodules for now.
Test Plan:
Create the following tests in the `sharness` directory:
```shell
$ cat sharness/good.t
#!/bin/sh
test_description='demo of passing tests'
. ./sharness.sh
test_expect_success "look at me go" true
test_expect_success EXPENSIVE "this may take a while" 'sleep 2'
test_done
# vim: ft=sh
$ cat sharness/bad.t
#!/bin/sh
test_description='demo of failing tests'
. ./sharness.sh
test_expect_success "I don't feel so good" false
test_done
# vim: ft=sh
```
Note that `yarn sharness` and `yarn test` fail appropriately. Note that
`yarn sharness-full` fails appropriately after taking two extra seconds,
and `yarn test --full` runs the latter. Each failure message should
print the name of the failing test case, not just the suite name, and
should indicate that the passing tests passed.
Then, remove `sharness/bad.t`, and note that the above commands all
pass, with the `--full` variants still taking longer.
Finally, remove `sharness/good.t`, and note that the above commands all
pass (and all pass quickly).
wchargin-branch: add-sharness
Summary:
This subtree has no effect on the new build process; it contains only
stale code.
Test Plan:
Running `yarn test --full` passes. Running `yarn build` and running an
HTTP server on the result indicates the expected behavior, as does
running `yarn start`. A quick `git grep public` finds no amok results.
wchargin-branch: remove-public
Summary:
We were asking the `clean-webpack-plugin` to remove the `build/`
directory in all cases. However, Webpack accepts a command-line
parameter `--output-path`. When such a parameter is passed, we would be
removing the wrong directory.
The proper behavior is to remove “whatever the actual output path is”.
Webpack exposes this information, but it appears that the
`clean-webpack-plugin` does not take advantage of it. Therefore, this
commit includes a small Webpack plugin to do the right thing.
Test Plan:
Test that the behavior is correct when no output directory is specified:
```
mkdir -p build && touch build/wat && yarn build && ! [ -e build/wat ]
```
Test that the behavior is correct with an explicit `--output-path`:
```
outdir="$(mktemp -d)" && touch "${outdir}/wat" && \
yarn build --output-path "${outdir}" && \
! [ -e "${outdir}/wat" ]
```
Test that the plugin refuses to remove the root directory:
```
! yarn build --output-path . && \
sed -i '/path: /d' config/makeWebpackConfig.js && ! yarn build
```
(Feel free to comment out the actual `rimraf.sync` line in the plugin
when testing this.)
wchargin-branch: clean-actual-build-directory
Summary:
Running `yarn test` (equiv. `npm test` or `npm run test`) now runs all
checks. It takes the place of the former `yarn travis`. This is more in
line with the expectation of a top-level `test` command: if it passes,
your code is good.
The `unit` command now runs Jest once, not in watch mode. It takes the
place of the former `ci-test`. To run tests in watch mode, run any of
the following:
- `yarn unit --watch`, or
- `npm run unit -- --watch`, or
- `npm unit -- --watch`.
This behavior is more consistent with the standard behavior of commands
like `make test`. It is also empirically what @wchargin and
@decentralion want most of the time.
Test Plan:
Verify that each of the scripts `test`, `unit`, and `coverage` passes.
Verify that each of the aforementioned `--watch` invocations works.
Verify that `.travis.yml` has the correct `script:` command.
wchargin-branch: reorganize-test-command
William Chargin
Dandelion Mané
