This will allow timeline cred (#862) to do a better job of flowing cred
across reaction edges. (Very old reactions should not be moving a lot of
present-day cred.)
Test plan: Inspected snapshot changes.
I added `mentionsAuthorReference` based on an untested hypothesis that
they would be useful. With the passage of time, I've never seen any
evidence that they actually improve cred socres (their impact seems
negligible), and they add complexity.
In the future, "go-fishing" style heuristics like this should not merge
unless they are of clearly demonstrated value. Also, it would be better
to add stuff like this via a standalone plugin rather than in the core
GitHub logic.
Undoes #806.
Test plan: `yarn test`
As of the timeline cred work, I'm shifting emphasis away from raw
PageRank results, in favor of timeline pagerank results. As such,
there's no need to have load save the regular pagerank results on
creation.
As of #1136, there will be no need for timestampMap, as that data will
be present directly in the graph. As the timeline cred UI will depend on
the full graph for analysis, let's save the graph instead.
Test plan: `yarn test` and snapshot inspection.
At present, the Git commit node type lives in a strange state of shared
responsibility between GitHub and Git. The Git plugin is nominally
responsible for it, but its render method tries to show a hyperlink to
GitHub -- which is awkward for many reasons, including that the same Git
commit could have multiple hyperlinks on GitHub.
This commit resolves that issue by separating the existing commit type
into two: the Git Commit type, which is owned by the Git plugin and
doesn't have hyperlinks or any fancy GitHub metadata, and the GitHub
Commit, which is owned by the GitHub plugin, corresponds to a unique
database id in GitHub, and has a corresponding GitHub url.
The two commits are connected by a CorrespondsToCommit edge type, which
links from the GitHub commit to the corresponding Git commit.
This is necessary for #1136, as if we want to make descriptions a part
of the graph payload, we need for descriptions to be unique for a given
address--and descriptions are only unique if we identifiy each GitHub
commit pointer as a separate address.
Test plan: The unit testing in this part of the codebase is light, so I
verified that the frontend work as expected for `sourcecred/sourcecred`
and `sourcecred/research`. The new node type and edge type appear
properly in the UI, the GitHub commits are connected to their Git
counterparts, etc.
This modifies `sourcecred load` so that it saves timestamp information
for all of the loaded plugins in a single aggregated map.
This is quite convenient, as it saves consumers of timestamp information
from needing to worry about the (rather hacky) implementation whereby
the data is fed from each adapter. Instead, consumers can just load the
timestamp map. This will also make it much easier to use timestamp info
in the research codebase.
Test plan: The timestampMap module has testing around generating the map
from the adapter and nodes, writing it, and reading it.
I haven't added any testing to the `load` CLI command. I think it would
be redundant as the updated snapshot test reveals that the map is
getting serialized properly.
Tests pass, and I have inspected the snapshot
As SourceCred has evolved, we've grown more and more snapshot tests that
are not included in Jest. The GitHub plugin has two ad-hoc snapshot
tests, the Git plugin has one, and the sharness test suites have one.
This makes it difficult to keep track of where to update snapshots when
core changes are made. To fix this, I've added a script,
`scripts/update_snapshots.sh`, which updates snapshot tests across the
project.
Test plan: I removed existing snapshots across the codebase, ran the
snapshot tester, and they correctly regenerated.
Thanks to @wchargin for [catching it].
[catching it]: https://github.com/sourcecred/sourcecred/issues/1151#issuecomment-494256526
Generated this change via:
```
$ yarn backend
$ (cd sharness; UPDATE_SNAPSHOT=1 ./test_load_example_github.t -l)
```
Test plan: `yarn test --full`, excluding the known (local-only) failure
described in #1151
Modifies the Git plugin so that we now track commit author dates.
Similar to in #1152, they are encoded in MsSinceEpoch.
Test plan: `yarn test --full` passes, except for the pre-existing
failure discussed in #1151.
Thanks to @s-ben for a conversation which motivated these changes.
Updates github schema to include createdAt timestamps, and then updates
the RelationalView to provide those timestamps as MsSinceEpoch.
I added createdAt timestamps to Repos, Issues, Pulls, Reviews, and
Comments, as these correspond to GitHub graph nodes where I think
time-based filtering is relevant. I didn't add them to Users, Reactions,
or Commits. Reactions, because they correspond to edges not nodes. (We
could consider doing the time filtering on edges too, but I'd rather
keep it simple for now.) Commits, because they're owned by a different
plugin. Users, because... in a certain sense the user identity is
timeless, the time factoring is mostly so we can evaluate how users'
cred varies over time.
Anyway, it will be easy to add more fields later if we need them.
Test plan:
- Inspect snapshot changes
- Ran `yarn test --full`
- Its only failure is pre-existing, per #1151
Thanks to @s-ben for some motivation and discussion about this change.
This commit updates the `sourcecred load` command so that it also
automatically runs PageRank on completion.
The implementation is slightly hacky, in that it prints two sets of
task status headers/footers to console, for reasons described in a
comment in the source code. The user-visible effect of this hack can
be seen below:
```
❯ node bin/sourcecred.js load sourcecred/example-github
Starting tasks
GO load-git
GO load-github
DONE load-github
DONE load-git
Overview
Final result: SUCCESS
Starting tasks
GO run-pagerank
DONE run-pagerank
Overview
Final result: SUCCESS
```
It would be good to clean this up, but for now I think it's acceptable.
Note that it is not safe to assume that a PagerankGraph always exists
for repos that are included in the RepoIdRegistry. The repo gets added
to the registry before the pagerank task runs. Consumers that are
loading the `PagerankGraph` can just check that the file exists, though.
Test plan: I've added unit tests that verify that the right tasks are
generated. Most importantly, the snapshot of the results of `sourcecred
load` now include a snapshotted pagerank graph.
(The snapshot was updated via `UPDATE_SNAPSHOT=1 yarn test --full`.)
Further progress on #967.
Updating github example data with support
for 🚀 and 👀 reaction types.
This follows #1068 and @decentralion updating
the archived repo with the new reaction types.
`src/plugins/github/fetchGithubRepoTest.sh -u`
(as @decentralion suggested) updated `example-github.json`
`yarn unit` caught two tests with failing snapshot
tests (`createGraph.test` and `relationalView.test`), so
I updated those with `yarn unit -u`
`yarn test -full` caught a failing snapshot test
at `sharness-full`, resolved by updating the
snapshot in `view.json.gz` with
`UPDATE_SNAPSHOT=1 yarn test --full`.
Thanks to @wchargin for the [explanation] on how
to resolve that issue.
[explanation]: https://github.com/sourcecred/sourcecred/pull/1077#pullrequestreview-196805017
**Test Plan:**
`yarn test --full` is passing.
Additionally, the commands:
```sh
filepath="./sharness/__snapshots__/example-github-load/data/sourcecred/example-github/github/view.json.gz" &&
[ -f "${filepath}" ] && # sanity check
diff -u \
<(git show "HEAD:${filepath}" | gzip -d | jq .) \
<(gzip -dc "${filepath}" | jq .) \
;
```
yields the following output:
```
--- /dev/fd/63 2019-01-27 08:34:15.020387301 -0500
+++ /dev/fd/62 2019-01-27 08:34:15.021095696 -0500
@@ -654,6 +654,22 @@
"subtype": "USER",
"login": "decentralion"
}
+ },
+ {
+ "content": "ROCKET",
+ "user": {
+ "type": "USERLIKE",
+ "subtype": "USER",
+ "login": "decentralion"
+ }
+ },
+ {
+ "content": "EYES",
+ "user": {
+ "type": "USERLIKE",
+ "subtype": "USER",
+ "login": "decentralion"
+ }
}
]
}
```
Again, thanks @wchargin's for providing those commands and accompanying
explanation.
Summary:
Our registry was defined to simply be a list of IDs. This is
insufficiently flexible; we want to be able to annotate these IDs with,
e.g., last-updated times (#989). This commit wraps the entries in a
simple object, updating clients appropriately.
Test Plan:
- Run `node ./bin/sourcecred.js load sourcecred/example-github` with a
repository registry in the old format, and note that it errors
appropriately.
- Run `yarn build` with a repository registry in the old format, and
note that it errors (“Compat mismatch”).
- Delete the old registry and re-run the `load` command. Note that it
runs successfully and outputs a registry. Run `yarn build`; note
that this works.
- Load data for two repositories. Run `yarn start`. Note that the list
of prototypes still works, and that you can navigate to and render
attributions for individual project pages.
- Verify that `yarn test --full` passes.
wchargin-branch: repo-id-registry-metadata
We added a configurable cred feedback url on the theory that we would
create a separate discourse post to collect feedback for each project in
particular.
We've now realized that no one is using this, so it's just vestigial
complexity now. So I'm removing the logic for configuring the feedback
url on a per-project basis.
Instead, we will always link to a Google form for collecting feedback.
Test plan: `yarn test --full` passes, and I manually checked the links.
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:
Fixes#955. Our test runner does run `yarn backend` before Sharness
tests, but it builds the backend applications into a temporary directory
rather than squashing the repository backend (which is good!). The test
script has learned about this directory.
Test Plan:
Run `rm -rf ./bin && yarn test --full`, which fails before this commit
and passes after it.
wchargin-branch: fix-test-load-example-github
Summary:
This is a quick patch for #955, pending investigation and fix.
Test Plan:
```shell
$ cd sharness/
$ ./test_load_example_github.t --long
ok 1 - should load sourcecred/example-github
ok 2 # skip should update the snapshot (missing UPDATE_SNAPSHOT of !CIRCLECI,LOADED_GITHUB,UPDATE_SNA
PSHOT)
ok 3 - should be identical to the snapshot
# passed all 3 test(s)
1..3
$ CIRCLECI=true ./test_load_example_github.t --long
ok 1 # skip should load sourcecred/example-github (missing !CIRCLECI of !CIRCLECI,EXPENSIVE,HAVE_GITH
UB_TOKEN)
ok 2 # skip should update the snapshot (missing UPDATE_SNAPSHOT,LOADED_GITHUB,!CIRCLECI of !CIRCLECI,
LOADED_GITHUB,UPDATE_SNAPSHOT)
ok 3 # skip should be identical to the snapshot (missing LOADED_GITHUB,!CIRCLECI of !CIRCLECI,LOADED_
GITHUB)
# passed all 3 test(s)
1..3
```
Ref.: <https://circleci.com/docs/2.0/env-vars/#built-in-environment-variables>
wchargin-branch: test-skip-failing-circleci
This will enable us to test code that needs to consume the results of
running `sourcecred load`, e.g. plugin adapter code.
If you need to update the snapshot, run
(cd sharness; UPDATE_SHAPSHOT=1 ./test_load_example_github.t)
Test plan: `yarn sharness-full` passes.
Paired with @wchargin
Summary:
Resolves#800. The newly added test takes about 2ms per file.
Test Plan:
Run `yarn sharness`, and note that it passes.
Then, edit (say) `src/main/test.js` to change the top-level describe
block from `"cli/main"` to something else, or to remove it altogether.
Re-run `yarn sharness` and note that it fails with a helpful message:
```
test_js_tests_have_top_level_describe_block_with_filename.t .. 1/?
not ok 31 - test file: cli/main.test.js
test_js_tests_describe_filename.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/65 subtests
```
wchargin-branch: describe-test
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 patch fixes a particularly sneaky bug. Our test script contains a
literal backtick inside single quotes. This is generally not a problem,
because backticks inside single quotes do nothing. But the contents of
the single quotes are interpreted as Bash by our test runner, and at
that time the single quotes are expanded to a command substitution.
Therefore, `grep` is invoked as if writing
grep -e "warning: running $(yarn backend)"
at the CLI. This will actually invoke `yarn backend`!
The magnificent aspect of this bug is that it both makes the test script
slower by about ten seconds _and_ completely and silently defeats the
assertion in which it’s contained. The output of `yarn backend` contains
several blank lines. Therefore, one of the literal patterns to `grep`
contains a blank line. This causes `grep` to match _every_ line in the
error file, regardless of whether it is one of the intended messages.
This patch is the 666th PR to SourceCred. In my opinion, it deserves
this dubious honor.
Test Plan:
Note that `yarn test --full` works, but fails if one of the expected
error message patterns is deleted or munged.
Confirm the behavior by prepending `echo backend >>/tmp/log &&` to the
`yarn backend` script in `package.json`, noting that the resulting log
file contains four lines before this patch and two lines after it.
(Don’t forget to delete/clear the log file before invocations.)
Confirm the behavior of `grep` by writing:
```shell
$ printf 'things went wrong!\n' >err
$ printf 'wat\n\nwot\n' >patterns
$ grep -vF -e "okay" -e "warn: `cat patterns`" err; echo $?
1
$ printf 'wat\nwot\n' >patterns # no empty line
$ grep -vF -e "okay" -e "warn: `cat patterns`" err; echo $?
things went wrong!
0
```
wchargin-branch: fix-build-test-quoting
Summary:
This change should have happened in #768. However, I didn’t catch it
then because `yarn test --full` passes even before this commit, despite
the expected error being clearly wrong! It turns out that a very sneaky
bug conspires with this one to result in the test passing no matter what
kinds of warnings `yarn backend` may output. This bug is fixed in #772.
Test Plan:
Observe that the error message is now correct by comparing against the
source in `config/RemoveBuildDirectoryPlugin.js`. Then, apply #772 and
note that `yarn test --full` still passes, but does not pass when #772
is applied and this change is reverted.
wchargin-branch: fix-expected-error-message
Summary:
In #715, I used Bash arrays for convenience. Our tests should run under
POSIX `sh` (as on Travis and standard GNU/Linux). This patch
reimplements the check using only POSIX features.
Fixes#752.
Test Plan:
As is, `yarn test --full` passes on GNU/Linux and macOS(+GNU coreutils).
Change the glob from `main.*.js` to `*.js` and note that running the
test emits an error:
```
fatal: multiple main bundles found:
build_output/output_NO_REPOS/static/js/main.6307f660.js
build_output/output_NO_REPOS/static/js/ssr.e92af807.js
```
Change the glob from `main.*.js` to `nope.*.js` and note that running
the test emits an error:
```
fatal: no main bundle found
```
Revert the glob to normal and note that all tests run and pass.
(To run tests, `./test_build_static_site.t --chain-lint --long -v` from
the `sharness/` directory.)
wchargin-branch: posix-bundle-check
Test Plan:
Running `./test_build_static_site.t --long -v` no longer detects the
feedback URL as unsafe. (Prior to this commit, it emitted a message to
this effect.) The build is still broken on Linux for other reasons, but
works on macOS or any other system where `sh` resolves to Bash.
As a regression test, the “potentially unsafe argument” warning has been
made to actually fail the test case. To verify this, remove `:` from the
list of `unusual_chars`, run the test, and note that it fails outright.
wchargin-branch: shell-safe-colon
Summary:
We store the relational view in `view.json.gz` instead of `view.json`,
taking advantage of the isomorphic `pako` library for gzip encoding and
decoding.
Sample space savings (note that post bodies are included; i.e., #747 has
not been applied):
SAVE OLD (B) NEW (B) REPO
89.7% 25326 2617 sourcecred/example-github
82.9% 3257576 555948 sourcecred/sourcecred
85.2% 11287621 1665884 ipfs/js-ipfs
88.0% 20953425 2520358 gitcoinco/web
84.4% 38196825 5951459 ipfs/go-ipfs
84.9% 205770642 31101452 tensorflow/tensorflow
<details>
<summary>Script to generate space savings output</summary>
```shell
savings() {
printf '% 7s % 11s % 11s %s\n' 'SAVE' 'OLD (B)' 'NEW (B)' 'REPO'
for repo; do
file="${SOURCECRED_DIRECTORY}/data/${repo}/github/view.json.gz"
if ! [ -f "${file}" ]; then
printf >&2 'warn: no such file %s\n' "${file}"
continue
fi
script="$(sed -e 's/^ *//' <<EOF
repo = '${repo}'
pre_size = $(<"${file}" gzip -dc | wc -c)
post_size = $(<"${file}" wc -c)
percentage = '%0.1f%%' % (100 * (1 - post_size / pre_size))
p = '% 7s % 11d % 11d %s' % (percentage, pre_size, post_size, repo)
print(p)
EOF
)"
python3 -c "${script}"
done
}
```
</details>
Closes#750.
Test Plan:
Comparing the raw old version with the decompressed new version shows
that they are identical:
```
$ <~/tmp/sourcecred/data/sourcecred/example-github/github/view.json \
> shasum -a 256 -
63853b9d3f918274aafacf5198787e18185a61b9c95faf640a1e61f5d11fa19f -
$ <~/tmp/sourcecred/data/sourcecred/example-github/github/view.json.gz \
> gzip -dc | shasum -a 256
63853b9d3f918274aafacf5198787e18185a61b9c95faf640a1e61f5d11fa19f -
```
Additionally, `yarn test --full` passes, and `yarn start` still loads
data and runs PageRank properly.
wchargin-branch: gzip-relational-view
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 current version of the build script has the safe but annoying
property that the target directory must be an existing, empty directory.
It seems reasonable and convenient to allow the build script to create
the directory with `mkdir -p`. It still fails if the directory is not
empty or is a file.
Test Plan:
Unit tests updated; run `yarn sharness-full`.
wchargin-branch: build-mkdir-p
Summary:
This is copied from the Sharness repository’s `test/.gitignore`, and is
also the same as Git’s `t/.gitignore` minus a Git-specific exclusion.
Suggested by @decentralion.
Test Plan:
Run `yarn sharness` and interrupt it (SIGINT) while `make` is running.
Note that `sharness/` now contains a `trash directory.*`, which Git
ignores.
wchargin-branch: sharness-gitignore
Thanks to #642, it should now be safe to disable the Git plugin, reaping
the benefits described in #628, without causing the cred explorer to
crash (#631).
Test plan:
- `yarn travis --full` passes
- The full cred explorer works:
- Running PageRank does not crash the explorer
- Expanding a pull request does not crash the explorer
- (After clearing state) the weight config doesn't show Git weights
- The filter doesn't show Git nodes
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
This reverts commit 8c70f03122.
Context: This introduced a serious bug (#631), so we're reverting it to
get the codebase back in a working state. Meanwhile, I'll develop a
principled solution.
Test plan:
I rebuilt the backend, re-loaded a graph, and loaded it in the frontend.
PageRank, the cred explorer, and the weight config all work. Opening a
pull request does not trigger a crash.
See #627 for context.
Removing the Git plugin results in an enormous performance improvement.
In my testing on `metamask/metamask-extension`: before this change, load
took 23s, and PageRank took >9 mins and then crashed. After this change,
load+PageRank took 5s combined.
Test plan: All unit tests pass; loading new data from the CLI works; and
I poked around the UI to make sure there were no spurious references to
the Git plugin.
Note: This does not break backcompat, there's no need to regenerate any
already-loaded data.
Summary:
The `node ./bin/sourcecred.js load` command invokes plugin code by
providing an output directory into which the plugin may store data.
As of this patch, it also provides a cache directory that the plugin may
use to store data that will not be available at runtime. For instance,
the Git plugin might choose to clone the repository herein, or the
GitHub plugin may choose to store partial GraphQL query results to deal
with interruptions. The contract is that the cache directory may be
removed at any time and that the plugin should continue to operate
normally.
Test Plan:
The build script has been updated and tested. Reverting the change to
the build script causes the newly added test to fail. (Each plugin has a
cache directory, though the cache directories are empty for now.)
wchargin-branch: create-plugin-cache
Summary:
Currently, we create the static site and deploy it all at once in
`scripts/deploy.sh`. This commit creates a new script that only builds
the static site. This has the advantage that it is easier/less scary to
change that script (because it can be tested without worrying about
deploying to a local test target), and that we can write automated tests
for it.
Test Plan:
Run `yarn sharness`; note that it completes very quickly. Then, in a
shell with your GitHub token exported, run `yarn sharness-full`. Expect
all tests to pass.
For a sanity check, you can run:
```shell
outdir="$(mktemp -d --suffix .sourcecred-site)"
./scripts/build_static_site.sh --target "${outdir}" \
--cname sourcecred.io \
--repo sourcecred/example-git \
--repo sourcecred/example-github \
;
(cd "${outdir}" && python -m SimpleHTTPServer)
```
and ensure that <http://localhost:8000/> is as expected.
One test case that is not covered is the following: _if_ the actual app
somehow tries to emit a `CNAME` file at root, _and_ our script’s logic
to catch this is broken, then we will not catch this failure. I’ve
tested the logic manually by adding `>"${cname_file}"` after definition
of that variable, but I don’t see a good way to test it automatically,
without adding flags like `--but-actually-emit-cname-too` to the build.
The compound probability of this happening is sufficiently low that this
doesn’t bother me.
wchargin-branch: build-static-site-script
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
Dandelion Mané
Brian Litwin
William Chargin
