Skip to content

Conversation

Benaiah
Copy link
Contributor

@Benaiah Benaiah commented Jul 17, 2020

Which problem is this pull request solving?

Allows turning off sending plugin status to API server for development.

Describe the solution you've chosen

Previously, testOpts.sendStatus only worked if the mode was not buildbot. This makes that CLI flag work regardless of whether netlify-build is running in buildbot mode.

Checklist

Please add a x inside each checkbox:

  • I have read the contribution guidelines.
  • The status checks are successful (continuous integration). Those can be seen below.

@Benaiah Benaiah added the type: chore work needed to keep the product and development running smoothly label Jul 17, 2020
@ehmicky ehmicky self-requested a review July 18, 2020 11:13
Copy link
Contributor

@ehmicky ehmicky left a comment

Choose a reason for hiding this comment

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

The CI tests are failing due to the following problem: testOpts.sendStatus defaults to false, so this change would prevent the createPluginRun API request from being performed in production.

This can be fixed though by:

  • Making testOpts.sendStatus default to true instead (done here).
  • Setting its default value to false in automated tests (done here). Otherwise, all automated tests would start calling the createPluginRun API request, which would make them fail.
  • Making sure the tests related to createPluginRun are still passing in this new setup, since they explicitly set testOpts.sendStatus to true.

Also, since this CLI flag is useful outside automated tests, would it might make sense to rename it from testOpts.sendStatus to sendStatus in a follow-up PR?

@Benaiah
Copy link
Contributor Author

Benaiah commented Jul 20, 2020

I'm still having trouble getting the snapshot tests to work correctly 😞. It looks like a few of the tests are no longer showing the createPluginRun failures that they're supposed to. I'm working on getting them updated but I can't figure out what's going on.

@Benaiah
Copy link
Contributor Author

Benaiah commented Jul 20, 2020

It also looks like the package-lock.json files are being updated when I run npm run format even though I haven't changed the package.json files at all. Not sure what that's about either.

@Benaiah Benaiah force-pushed the allow-turning-off-plugin-status branch from 1a4bd63 to 95c18ea Compare July 20, 2020 18:21
@ehmicky
Copy link
Contributor

ehmicky commented Jul 20, 2020

The second problem is due to #1684 (this just needs a code review).
For the first problem, let's chat on Slack to see if I can help.

@ehmicky
Copy link
Contributor

ehmicky commented Jul 20, 2020

We found a solution and will implement this tomorrow:

  • Rename --testOpts.sendStatus to --sendStatus
  • Pass the --sendStatus CLI flag from the buildbot to @netlify/build
  • Remove the fact that mode must be buildbot to send statuses (use --sendStatus only instead)

@ehmicky
Copy link
Contributor

ehmicky commented Aug 4, 2020

This is implemented now. Should this PR be closed?

@Benaiah Benaiah closed this Aug 7, 2020
@Benaiah Benaiah deleted the allow-turning-off-plugin-status branch August 7, 2020 00:41
This was referenced Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore work needed to keep the product and development running smoothly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants