This commit adds PluginWeightConfig, which is responsible for
adding all the weights for an individual plugin. The top-level
WeightConfig now creates multiple PluginWeightConfigs. It also takes
responsibility for hiding the FallbackPlugin.
Test plan: The PluginWeightConfig is tested (and fairly simple). The
top-level WeightConfig is not yet tested (#604), so I manually tested
that the weights in the app still function.
Summary:
This logic isn’t currently useful to us. If we want this functionality,
we should consider using a Webpack plugin like `size-limit`. In the
meantime, this removes functionality from `scripts/backend.js`,
continuing on the path to #765.
Recommend reviewing with `-w`.
Test Plan:
Run `yarn backend` and `yarn backend --dry-run`, noting that each works.
wchargin-branch: backend-remove-file-sizes
Summary:
As of #775, this is no longer used.
Test Plan:
A `git grep eslint-loader` shows no results, and `yarn test --full`
passes.
wchargin-branch: remove-eslint-loader
Summary:
Webpack will fail (quickly) if any required entry points do not exist.
The `scripts/backend.js` script superfluously checks this, too. This
patch removes that check.
Test Plan:
In `config/paths.js`, change `src/cli/main.js` to `src/cli/wat.js`.
Then, `yarn backend`, and note that Webpack fails quickly, with an error
“entry module not found”. Note that Webpack fails equally quickly if you
change the path of the last entry point rather than the first one.
wchargin-branch: backend-remove-exists-check
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
`testUtil.configureEnzyme` now additionally asserts, after every test,
that `console.error` and `console.warn` were not called. Tests that
explicitly expect such calls can still be written by manually re-mocking
the relevant console method (and several examples already exist).
The code that explicitly specifies this for various enzyme test files
has been removed.
Test plan: `git grep "not.toHaveBeenCalled"` shows only unrelated usage.
`yarn test` passes. Adding a spurious console.warn to a passing test
causes it to fail.
Fixes#668
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:
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
Currently, it's possible to lock the cred explorer UI via the following
sequence of actions:
1. Press the analyze cred button
2. While it is running, modify the weights
After following this repro, the UI is stuck: it will say "loading"
forever, and the analyze cred button is disabled.
The issue is that loadGraph and runPagerank do not apply their success
(or failure) state transitions if they are pre-empted by another state
change. If a repo change occurs, that's the right behavior: the repo
change puts the state back to `"READY_TO_LOAD_GRAPH"`, which means the
UI is ready to re-load, and showing the PageRank results for the wrong
repo would be very confusing.
However, if an edge evaluator change occurs while loadGraph or
runPagerank is happening, the state is left in the "LOADING" status
(which means the analyze cred button is disabled).
This commit fixes the issue via a refactor: per @wchargin's suggestion,
responsibility for the edge evaluator moves from the state module out to
`credExplorer/App.js`. This dramatically simplifies the state module, as
we no longer need a `Substate` concept: we can simplify the state into a
single sequence of states.
As of the refactor, the bug is impossible.
Test plan: Unit tests have been updated to maintain coverage on the
refactored code. I manually tested that the bug no longer repro's, and
that the cred explorer UI continues to function. I did not add a new
test to protect against regression, because in the new codepath, the bug
is basically impossible.
This commit modifies the edge type so that it has a
`defaultForwardWeight` and `defaultBackwardWeight`, and these defaults
are respected by the `WeightConfig`.
I came up with reasonable-seeming defaults for the Git and GitHub
plugins; these will undoubtably be more methodically tuned in the
future.
Test plan: `yarn test` passes, also opening the cred explorer now has
the specified default weights in the WeightConfig. (Note that the
forward/backward directions are reversed as described in #749.)
This commit introduces a new component, `EdgeTypeConfig`, which is
responsible for configuring the weights for a given edge type. The
config creates two `WeightSlider`s: one for the forward direction, and
one for the backward direction. The `DirectionalitySlider` is no longer
used, and is removed. This fixes#596.
So as to avoid confusion, we now describe every edge with variables, as
in 'α REFERENCES β', and clarify that the weight modifies how cred flows
from β to α. This necessitated the creation of an `EdgeWeightSlider`,
local to the `EdgeTypeConfig`, which sets up a `WeightSlider` with the
necessary greek characters.
The EdgeTypeConfig is tested, so this is continuing progress towards
solving #604.
Test plan: I manually verified that modifying edge weights has the
expected effect on cred scores. Also, some new unit tests are included.
This factors `NodeTypeConfig` out of the `WeightConfig` component. The
scope for a `NodeTypeConfig` is that it configures a single node type.
Right now it just renders a single `WeightSlider`, but I like factoring
out both for consistency with the `EdgeTypeConfig` (see #749) and
because I expect we may want to add more complexity later.
Test plan: The new component has some tests, also I manually tested the
frontend.
* StateTransitionMachine.loadGraph reports success
Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.
Test plan: Unit tests included.
* Add StateTransitionMachine.loadGraphAndRunPagerank
This methods combines `loadGraph` and `runPagerank` into one method
which internally chains the two method. `runPagerank` is only called if
`loadGraph` was successful.
Progress on #586.
Test plan:
The new method has attached unit tests. I implemented the unit tests via
mocking, which seemed quite convenient as the method is basically a
wrapper for chaining two other function calls.
* Combine loadGraph and runPagerank into one button
Resolves#586. The new button is called "Analyze cred".
Test plan: Unit tests, also I tested it manually.
* StateTransitionMachine.loadGraph reports success
Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.
Test plan: Unit tests included.
* Add StateTransitionMachine.loadGraphAndRunPagerank
This methods combines `loadGraph` and `runPagerank` into one method
which internally chains the two method. `runPagerank` is only called if
`loadGraph` was successful.
Progress on #586.
Test plan:
The new method has attached unit tests. I implemented the unit tests via
mocking, which seemed quite convenient as the method is basically a
wrapper for chaining two other function calls.
Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.
Test plan: Unit tests included.
Fixes#732; see that issue for context.
Test plan:
The success case still works (verified that loading
sourcecred/sourcecred works).
I haven't tested the error case, as getting a real RATE_LIMIT_EXCEEDED
form GitHub is time-consuming, and has only happened once in practice.
I'm pretty confident the code works because it's a simple adaptation of
the code that catches other cases.
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
Test Plan:
Note that `yarn backend; node bin/sourcecred.js help` still works.
Note that `git grep -i oclif` returns no results.
Rejoice.
wchargin-branch: remove-oclif
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 ports the OClif version of `sourcecred load` to the sane CLI
system. The functionality is similar, but the interface has been
changed a bit (mostly simplifications):
- The `SOURCECRED_GITHUB_TOKEN` can only be set by an environment
variable, not by a command-line argument. This is standard practice
because it is more secure: (a) other users on the same system can
see the full command line arguments, but not the environment
variables, and (b) it’s easier to accidentally leak a command line
(e.g., in CI) than a full environment.
- The `SOURCECRED_DIRECTORY` can only be set by an environment
variable, not by a command-line argument. This is mostly just to
simplify the interface, and also because we don’t really have a good
name for the argument: we had previously used `-d`, which is
unclear, but `--sourcecred-directory` is a bit redundant, while
`--directory` is vague and `--sourcecred-directory` is redundant.
This is an easy way out, but we can put the flag for this back in if
it becomes a problem.
- The `--max-old-space-size` argument has been removed in favor of a
fixed value. It’s unlikely that users should need to change it.
If we’re blowing an 8GB heap, we should try to not do that instead
of increasing the heap.
- Loading zero repositories, but specifying an output directory, is
now valid. This is the right thing to do, but OClif got in our way
in the previous implementation.
Test Plan:
Unit tests added, with full coverage; run `yarn unit`.
To try it out, run `yarn backend`, then `node bin/cli.js load --help` to
get started.
I also manually tested that the following invocations work (i.e., they
complete successfully, and `yarn start` shows good data):
- `load sourcecred/sourcecred`
- `load sourcecred/example-git{,hub} --output sourcecred/examples`
These work even when invoked from a different directory.
wchargin-branch: cli-load
Summary:
This includes environment variables to specify the SourceCred directory
and the GitHub token. Parts of this may change once #638 is resolved.
Test Plan:
Unit tests included, with full coverage; run `yarn unit`.
wchargin-branch: cli-common
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:
This commit introduces the notion of a `Command`, which is simply a
function that takes command-line arguments and interacts with the real
world. This infrastructure will enable us to write a well-tested CLI.
The `Command` interface is asynchronous because commands like `load`
need to block on promise resolution (for loading GitHub and Git data).
This is annoying for testing, but does not actually appear to be a
problem in practice.
Test Plan:
Unit tests added. See later commits for real-world usage.
wchargin-branch: cli-command-infrastructure
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
Our serialized RelationalView can get quite large - in the case of
TensorFlow it's over 190MB. This is a problem, as GitHub pages have a
hard cap of 100MB on hosted files.
As a temporary workaround, this commit introduces a method,
`compressByRemovingBody`, which strips away the bodies of every post. In
the longer term, we'll need a solution that scales with larger
repositories, e.g. sharding the relational view into smaller pieces.
Test plan: Unit tests were added. I've manually confirmed that the
newly-generated views are smaller (2.1MB vs 3.3MB), and that the
frontend continues to function.
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:
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
The GitHub regex in urlIdParse.js incorrectly disallowed repo names with
underscores and dots. Fixes#721.
To mitigate errors like this in the future, code which uses regexes to
find owners and repos has been modified to all depend on the same regex
pattern.
Test plan:
Unit tests have been updated to include the failure case (they correctly
failed), and then code was updated so that the tests pass again.
Also, I manually verified that loading ipfs/js.ipfs.io no longer fails.
Paired with @wchargin
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
This commit isolates all of the log-weight behavior in the weight
slider. That slider moves in log space, but the numbers printed and
passed around the WeightConfig code are now always in linear-space.
This should reduce confusion in the UI and for developers.
This commit contains two other improvements: (#588)
- Changes the (log space) range on the sliders from ±10 to ±5
- Change the order from slider, weight, name to name, slider, weight, so
that there is more visual separation between the name and the weight.
Test plan: Changes to the weight slider are tested. Changes to the
WeightConfig aren't (#604) so I manually tested the UI.
PluginAdapters and Node/Edge types are increasingly fundamental to the
cred explorer. Prior to this commit, we had no canonical demo
adapters/types, and we would create ad-hoc and messy adapters whenever
we needed them. This creates unnecessary repetition and lowers test
quality.
This commit creates a canonical demo adapter (loosely themed based on
the wonderful game [Factorio]) and refactors most existing test cases to
use the demo adapters. In particular, the horrible mess of pagerankTable
adapters has been removed.
[Factorio]: https://www.factorio.com/
I left `aggregate.test.js` untouched because I would have needed to
materially re-write the tests to port them over. I added a comment so
that if we ever do re-write those tests, we'll use the new demo
adapters.
Test plan: `yarn test` passes.
This commit factors the weight sliders used for both node and edge
weights into a shared WeightSlider component, and factors out the
direction slider used for edge weights into a DirectionalitySlider.
Both of these components are tested. This is a step towards #604.
Test plan:
The specific behaviors of the sliders are well tested. Since the weight
config as a whole is not tested, I manually verified by messing with the
weights that node weights, edge weights, and edge directionality all
affects the cred distribution as anticipated.
Summary:
We currently load trees and then throw them away later, because we don’t
get useful signal from them. We should consider not doing that. This
will be faster.
Test Plan:
```
$ time node bin/sourcecred.js load tensorflow/tensorflow --plugin git
real 0m33.512s
user 0m35.196s
sys 0m12.489s
```
Also, `yarn test --full` passes.
wchargin-branch: git-deforestation
Adds a link titled "what is this?" that points to my gentle introduction
to cred. Also, move the feedback link to be next to it and get rid of
the prototype disclaimer.
Test plan: Visual inspection, also a test was updated.
Summary:
This fixes a bug where, if the `SOURCECRED_DIRECTORY` environment
variable is set to `foo` but the `-d bar` flag is passed, then the
repository registry will be written under `foo` but the plugin data will
be loaded under `bar`.
Test Plan:
```
$ rm -rf /tmp/good /tmp/bad
$ SOURCECRED_DIRECTORY=/tmp/bad >/dev/null \
> node bin/sourcecred.js load sourcecred/example-github -d /tmp/good
$ [ -d /tmp/bad ]; echo $?
$ find /tmp/good
/tmp/good
/tmp/good/cache
/tmp/good/cache/sourcecred
/tmp/good/cache/sourcecred/example-github
/tmp/good/cache/sourcecred/example-github/github
/tmp/good/cache/sourcecred/example-github/git
/tmp/good/repositoryRegistry.json
/tmp/good/data
/tmp/good/data/sourcecred
/tmp/good/data/sourcecred/example-github
/tmp/good/data/sourcecred/example-github/github
/tmp/good/data/sourcecred/example-github/github/view.json
/tmp/good/data/sourcecred/example-github/git
/tmp/good/data/sourcecred/example-github/git/graph.json
```
wchargin-branch: load-pass-context
Adds a badge, adds a link, slight rewrite of the contributing section.
Test plan: Check the links and badge works.
(The badge topic count is off, we'll see if it is at least directionally
correct over time.)
Summary:
This improves the documentation for the change made in #715.
Test Plan:
Note that `./scripts/build_static_site.sh --help` and `yarn sharness`
pass.
wchargin-branch: feedback-url-help-text
Dandelion Mané
William Chargin
Claire L