Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Common questions at C&L events #86

Open
BridgeAR opened this issue Oct 11, 2018 · 7 comments
Open

Common questions at C&L events #86

BridgeAR opened this issue Oct 11, 2018 · 7 comments

Comments

@BridgeAR
Copy link
Member

BridgeAR commented Oct 11, 2018

This issue is meant as a kind of FAQ for mentees and mentors.

Please add comments for things that you might encounter so others can look this up as well.

Questions:

  • How to make sure the code is covered without running make coverage.
    • A simple solution is to just add a console.log statement at the code point that should be reached and to check the console output.
  • How to get the test output from console.log() in the Node core tests?
    • Just run the code with ./node ./test/parallel/test-foo.js instead of python tools/test.py parallel/test-foo.js.
      If the test file has a "flags" comment at the top, start Node.js with those manually, e.g., ./node --expose-internals ./test/parallel/test-foo.js.
  • How to run a single test or all tests from a specific sub system?
    • Either use ./node relative-or-absolute-test-file-path.js or python tools/test.py subsystem/file-name.js for a single file or python tools/test.py -J subsystem for a whole subsystem or python tools/test.py -J glob
  • How to create a new test?
    • Check already existing tests. The test passes if the file executes without error. So just write the test as any other regular code.
  • What should be done with the pull request check marks if there is no documentation involved?
    • Please just mark the check mark as done or remove the entry if not applicable.
  • What should be done if make test shows failing tests on a local machine having the Nodejs master branch checked out without any changes?
    • Please check if there is an open issue that already summarizes the issue on your platform. If not, please open a new issue on the main Nodejs repository and copy the output in that issue, add all system information you have and under what circumstances the test(s) failed. If you are able to provide additional information to an existing issue, please do that as well.
  • The pull request got a "author ready" label but the code is not immediately merged. Why not?
    • We try to merge C&L PRs fast but for some PRs we'll want to make sure more collaborators got the chance to look at it. Therefore there are some rules in place how long a pull request has to be open before it may land. It also has to have a full CI run that has to be green so we are able to land it. So please be patient. If there is no activity for multiple days, just ask for reviews.
  • TBD...
@ChALkeR
Copy link
Member

ChALkeR commented Oct 12, 2018

  1. [macOS] test-cluster-bind-privileged-port and test-cluster-shared-handle-bind-privileged-port fail on macOS Mojave, see failing tests on macos mojave node#21679.
    Solution: known Mojave issue, just ignore those two.
  2. [macOS] Tests that launch subprocesses and wait for them to abort (e.g. test-process-abort-exitcode and test-domain-with-abort-on-uncaught-exception) take long to run and/or timeout on macOS with CrashReporter enabled.
    Solution: disable CrashReporter, see Investigate flaky test/async-hooks/test-callback-error node#13527 (comment).
  3. Quite often, people have (slightly?) outdated master branch versions. git pull upstream master =).
  4. [Windows]: There was a windows-specific problem with installing a proper Visual Studio build tools. Installing just "Visual C++ build tools" (as https://github.com/nodejs/node/blob/master/BUILDING.md#windows-1 recommenda) didn't help. Installing some more Visual Studio-related stuff helped, but I don't have the details — I wasn't the one who resolved that.

@ChALkeR
Copy link
Member

ChALkeR commented Oct 12, 2018

Those issues are not code-and-learn specific, I think that they should be either resolved or mentioned in the documentation (e.g. BUILDING.md).

@mhdawson
Copy link
Member

Not necessarily 100% relevant but one comment I got was that its not always obvious who the mentors are. Something like wearing the same shirt or hats that stand out might help ?

So the question would be "who are the mentors"

@mcqj
Copy link

mcqj commented Nov 6, 2018

It would be useful if the tasks being assigned include the issue number of the associated issue so that committers can include a reference in their commit message.

@BridgeAR
Copy link
Member Author

BridgeAR commented Nov 6, 2018

@mcqj Icreated these tasks specifically for the code and learn event and there are no issues for them. It would have been even more work to create issues and I hope you all had a good experience also without such a reference!

@mcqj
Copy link

mcqj commented Nov 15, 2018

No problem at all @BridgeAR

@gireeshpunathil
Copy link
Member

  1. I completed my first PR, where do I go next? How do I get the list of next level PRs that I can make?

Please review https://www.nodetodo.org/next-steps/

  1. Did you make this (the code change requirement) on purpose, for the new comers?

No, these are incremental improvements that we identified but kept on stock for this exercise. We did not artifically created these issues for the purpose of code & learn.

  1. What happens if I raise PR on the master of my fork?

If you are raising a single PR no problem with that. When a second undertaking is being worked, the existing branch carries the first PR's content that is not desirable. So the best practise is to create one branch per PR and work inside that, for isolation and easy management.

  1. How to run single test?

Exaplained above.

  1. How do I know whether lint (or everything) passes? my output was too long, and at the end of it did not say success or failure.

If there is an error on any of the passes, the bottom of the log will clearly say so. If no error report found, that means all is well.

  1. I am new to git and github. Can you teach me that first?

Sure, will plan for that next time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants