Add Process object so as to allow process.env

[randomboi404]

This Pull Request fixes/closes #4492.

It changes the following:

1. Ability to access ENV variables passed into boa:

Example code

const api_key = process.env.API_KEY
API_KEY=xxxxx boa -m app.js

2. Allows future addition of other process object methods of node js.

[github-actions]

Test262 conformance changes

Test result	main count	PR count	difference
Total	52,598	52,598	0
Passed	49,420	49,421	+1
Ignored	2,134	2,134	0
Failed	1,044	1,043	-1
Panics	0	0	0
Conformance	93.96%	93.96%	+0.00%

Fixed tests (1):

test/staging/Temporal/v8/instant-to-json.js (previously Failed)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.49%. Comparing base (6ddc2b4) to head (77c459c).
⚠️ Report is 641 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4597      +/-   ##
==========================================
+ Coverage   47.24%   56.49%   +9.25%     
==========================================
  Files         476      547      +71     
  Lines       46892    60060   +13168     
==========================================
+ Hits        22154    33933   +11779     
- Misses      24738    26127    +1389     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hansl]

2 non-blocking nits for now. If you want to merge without addressing that's fine we can do it in a follow up PR.

[randomboi404]

That makes sense.

Regarding the new TestAction::harness() since console tests haven't been migrated yet, I think it's better to let process be consistent with it and do a follow up once test harness is adopted more broadly.

Regarding it being enabled by default, I think we should let it be enabled for now and if in case we strictly don't want it enabled as it's a node api, we'll do a follow up to make it not being enabled by default.

[nekevss]

I'd second having the Process extension not on by default.

I think this is a super cool extension to have as an option, but since process is not based on a web standard, we should have it be an opt in extension and not opt out.

[randomboi404]

Done! I've made it optional.

[nekevss]

Thanks! 😄

The feature flag approach looks good to me!

[jedel1043]

Thank you for the contribution! I have a couple of questions about the implementation

[jedel1043]

Wouldn't this cause UB if Cargo decides to run this test and the test below it concurrently?

[nekevss]

Oh, shoot. That's true.

It may be worth using something like temp_env as a dev-dependency.

[randomboi404]

Ah yes, my bad for overlooking it.
Also as per @nekevss's suggestion, I'll use temp_env as a dev dependency and fix it.

[randomboi404]

Done!

[jedel1043]

Unfortunately, I don't think this fixes the issue, according to this issue in the temp_env repo. I see two possible solutions:

• Require running tests with nextest, but it feels "weird" to depend on another test runner to not cause UB.
• Somehow run the test in a subprocess that sets up the environment variables before running it. This is the safest way of testing this but might be much more complex to set up.

Are you open for taking a shot at the second option? If not, then I would propose ignoring the tests for now and opening an issue for fixing this later.

[hansl]

@jedel1043 I think there is a couple other solutions:

1. have a global mutex lock. Rust describes that set_var is only UB when multiple threads try to write at the same time. If threads are synchronized, there's no UB.
2. merge the two tests into the same test to avoid this case. I don't recommend this one because we might add more tests in runtime with set_var later on and would likely not think about updating this test suite.

This saves having to create a subprocess which is heavy.

Ref https://doc.rust-lang.org/stable/std/env/fn.set_var.html#safety

[jedel1043]

Hmm a lot of options to discuss then.

[jedel1043]

In that case, let's just merge after ignoring the new tests, and we can discuss the shape of our solution later

[randomboi404]

I agree that the subprocess option would be safest but it'll be quite complex to implement for what we're trying to test here.

I think we can go for the mutex route later but I'm okay with ignoring the new tests for now and create a new issue for it where we can discuss it.

Thanks for the review!