Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

"npm run add add-simulated-synopsis-visits" does not work on master #11394

Closed
luixxiul opened this issue Oct 9, 2017 · 5 comments · Fixed by #12014
Closed

"npm run add add-simulated-synopsis-visits" does not work on master #11394

luixxiul opened this issue Oct 9, 2017 · 5 comments · Fixed by #12014
Assignees
Labels
0.19.x issue first seen in 0.19.x dev-setup feature/rewards priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/exclude

Comments

@luixxiul
Copy link
Contributor

luixxiul commented Oct 9, 2017

Test plan

#12014 (comment)


Description

npm run add-simulated-synopsis-visits does not work on master.

Uncaught Exception:
Error: ENOENT: no such file or directory, open '/Users/Suguru/Library/Application Support/brave-development/ledger-synopsis.json'
    at Object.fs.openSync (fs.js:584:18)
    at Object.module.(anonymous function) [as openSync] (ELECTRON_ASAR.js:168:20)
    at Object.fs.readFileSync (fs.js:491:33)
    at Object.fs.readFileSync (ELECTRON_ASAR.js:501:29)
    at updateExistingSynopsisFile (/Users/Suguru/browser-laptop/tools/lib/synopsisHelpers.js:26:45)
    at addSimulatedSynopsisVisits (/Users/Suguru/browser-laptop/tools/lib/utilApp/index.js:47:3)
    at App.app.on (/Users/Suguru/browser-laptop/tools/lib/utilApp/index.js:60:7)
    at emitTwo (events.js:111:20)
    at App.emit (events.js:194:7)

Steps to Reproduce

  1. npm run add-simulated-synopsis-visits

Actual result:

See above.

Expected result:

Reproduces how often: [What percentage of the time does it reproduce?]

Brave Version

about:brave info:

433e69f

Reproducible on current live release:

Additional Information

@bsclifton
Copy link
Member

Removed bug label- there is not a bug with Brave. This extra functionality for testing is broken after recent changes to the ledger. I believe with the reworking, ledger-synopsis.json was removed entirely

cc: @NejcZdovc

@bsclifton bsclifton added the priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). label Oct 9, 2017
@luixxiul
Copy link
Contributor Author

luixxiul commented Oct 9, 2017

The script is actually useful to test the ledger table (such as displaying Show Less button for example), so it would be appreciated if it is fixed soon.

@luixxiul luixxiul added the 0.19.x issue first seen in 0.19.x label Nov 1, 2017
@NejcZdovc NejcZdovc self-assigned this Nov 17, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Nov 17, 2017
Resolves brave#11394

Auditors:

Test Plan:
bsclifton added a commit that referenced this issue Nov 17, 2017
bsclifton added a commit that referenced this issue Nov 17, 2017
bsclifton added a commit that referenced this issue Nov 17, 2017
bsclifton added a commit that referenced this issue Nov 17, 2017
@luixxiul
Copy link
Contributor Author

checked on master and works nicely.

@kjozwiak
Copy link
Member

Re-opening as this doesn't seem to be working... @bsclifton also confirmed that it wasn't working for him.. STR:

  • checkout any of the branches
  • ensure that you've removed the node_modules folder
  • npm install && npm run watch
  • run brave via npm start and enable the wallet via about:preferences#payments
  • close brave once the wallet has been created
  • npm run add-simulated-synopsis-visits 100 within the browser-laptop
  • re-launch brave via npm start and you'll notice that the ledger table hasn't been filled under about:preferences#payments

Checked both macOS 10.13.1 x64 & Ubuntu 17.10 x64:

  • master branch: FAILED (didn't populate the table)
  • 0.21.x branch: FAILED (didn't populate the table)
  • 0.20.x branch: FAILED (didn't populate the table)
  • 0.19.x branch: FAILED (didn't populate the table)

@luixxiul when you ran this on Debian, did the ledger table get populated? Maybe we're doing something incorrectly?

@kjozwiak kjozwiak reopened this Nov 29, 2017
@NejcZdovc
Copy link
Contributor

this was broken with the latest bat libraries

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Nov 29, 2017
Fixed recovery promotional removal
Removes staging flag
Fixes fake visits

Resolves brave#12131
Resolves brave#12098
Resolves brave#11394

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Nov 29, 2017
Fixed recovery promotional removal
Removes staging flag
Fixes fake visits

Resolves brave#12131
Resolves brave#12098
Resolves brave#11394

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Nov 29, 2017
Fixed recovery promotional removal
Removes staging flag
Fixes fake visits

Resolves brave#12131
Resolves brave#12098
Resolves brave#11394

Auditors:

Test Plan:
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0.19.x issue first seen in 0.19.x dev-setup feature/rewards priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). QA/checked-Linux QA/checked-macOS QA/checked-Win64 QA/test-plan-specified release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants