Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add package config URI into VM test entrypoints #2245

Merged
merged 8 commits into from
Jun 27, 2024

Conversation

natebosch
Copy link
Member

@natebosch natebosch commented Jun 14, 2024

Towards #2246

This will allow a client of the VM service to evaluate the top level
variable and locate the package config used by the test runner.

Add a public testBootstapContents function in test_compiler.dart
which centralizes the bootstrap templates. Add an argument for the
package config URI and a VmTestType which is either "Vm" or
"Native".

Add the first test case in test_core. Add the pub dependency and
override for package:test, as well as the mono_repo configuration to
run unit tests. Other tests for test_core code is under the test
package but should be moved eventually.

This will allow a client of the VM service to evaluate the top level
variable and locate the package config used by the test runner.
@kenzieschmoll
Copy link

This solution works as intended (see flutter/devtools#7941). We should verify this also works from the flutter tools bootstrap generator (CC @christopherfujino)

@natebosch natebosch requested a review from jakemac53 June 17, 2024 22:16
@natebosch natebosch marked this pull request as ready for review June 17, 2024 22:16
@kenzieschmoll
Copy link

kenzieschmoll commented Jun 25, 2024

The flutter tools change for this solution is working as intended and awaiting review (flutter/flutter#150440). I recommend we add similar test coverage and code comments in this PR to prevent any future changes to this variable in the generated bootstrap.

Use `packageConfigUri` - we should already be validating a non-null
package config for platforms where this is used. Use a non-null
argument.

Don't reuse part of the args in the template - repeat the arg name in
`mainArgs` and `forwardedArgName`.

Add a comment on the variable to remind us that usage will not be
obvious.
Copy link
Contributor

@jakemac53 jakemac53 left a comment

Choose a reason for hiding this comment

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

Now that we have a separate public function, it should be reasonable to add a unit tests that it contains the expected variable.

LGTM once that is in

Add mono_pkg and pubspec configuration to run tests from the `test_core`
package.
@natebosch
Copy link
Member Author

We have a bunch of tests for test_core stuff under test, but I'm not going to keep spreading that pattern. I'll add the first set of tests, and configuration to run them, for test_core with the single test case for now. In the future we can migrate some of the tests from test into test_core.

@natebosch natebosch merged commit 4da62ac into master Jun 27, 2024
39 checks passed
@natebosch natebosch deleted the package-config-variable branch June 27, 2024 19:07
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jul 1, 2024
…, convert, crypto, csslib, dartdoc, fixnum, html, http, lints, logging, markdown, matcher, mime, mockito, path, pool, pub_semver, shelf, source_map_stack_trace, sse, stack_trace, stream_channel, string_scanner, term_glyph, test, test_descriptor, test_process, test_reflective_loader, tools, typed_data, vector_math, watcher, web, web_socket_channel, webdriver, webkit_inspection_protocol, yaml, yaml_edit

Revisions updated by `dart tools/rev_sdk_deps.dart`.

async (https://github.com/dart-lang/async/compare/d7c0cd5..c0d81f8):
  c0d81f8  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/async#280)

boolean_selector (https://github.com/dart-lang/boolean_selector/compare/62f82f6..c5468f4):
  c5468f4  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/boolean_selector#63)

browser_launcher (https://github.com/dart-lang/browser_launcher/compare/7348cea..6012690):
  6012690  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/browser_launcher#61)

cli_util (https://github.com/dart-lang/cli_util/compare/c37d5e1..6419270):
  6419270  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/cli_util#105)

clock (https://github.com/dart-lang/clock/compare/7cbf08e..ad428ea):
  ad428ea  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/clock#66)

convert (https://github.com/dart-lang/convert/compare/0c9eab7..9035caf):
  9035caf  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/convert#108)

crypto (https://github.com/dart-lang/crypto/compare/813e35e..1216790):
  1216790  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/crypto#175)

csslib (https://github.com/dart-lang/csslib/compare/b70fef2..192d720):
  192d720  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/csslib#205)

dartdoc (https://github.com/dart-lang/dartdoc/compare/7e5da60..e7f3694):
  e7f36946  2024-07-01  Sam Rawlins  Start using docImport resolution when resolving references (dart-lang/dartdoc#3805)
  9d756dc3  2024-06-30  Sam Rawlins  Bump to 8.0.10 (dart-lang/dartdoc#3806)
  4259b0b1  2024-06-28  Sam Rawlins  Parse and remove doc-imports from comment text (dart-lang/dartdoc#3803)
  31833c34  2024-06-28  Sam Rawlins  Correctly show extension type ctors and hide enum ctors (dart-lang/dartdoc#3804)
  2a39376a  2024-06-27  Sam Rawlins  Add the 8.0.9+1 changelog entry (dart-lang/dartdoc#3801)

fixnum (https://github.com/dart-lang/fixnum/compare/a8157d8..6c19e60):
  6c19e60  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/fixnum#132)
  57b41c2  2024-06-27  Kevin Moore  update lints (dart-lang/fixnum#131)

html (https://github.com/dart-lang/html/compare/f6c2c71..0da420c):
  0da420c  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/html#250)

http (https://github.com/dart-lang/http/compare/bf96551..8d89385):
  8d89385  2024-06-28  Brian Quinlan  Make it more clear to using use runWithClient in the Dart SDK. (dart-lang/http#1250)
  321362a  2024-06-27  Brian Quinlan  Document that runWithClient should not be used with flutter (dart-lang/http#1249)

lints (https://github.com/dart-lang/lints/compare/baaaa56..f6b5d36):
  f6b5d36  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart from 1.6.4 to 1.6.5 (dart-lang/lints#194)
  09a9c88  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.6 to 4.1.7 (dart-lang/lints#193)

logging (https://github.com/dart-lang/logging/compare/240ec33..6c3fb37):
  6c3fb37  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.6 to 4.1.7 (dart-lang/logging#168)

markdown (https://github.com/dart-lang/markdown/compare/3d8d7a8..6242437):
  6242437  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/markdown#617)

matcher (https://github.com/dart-lang/matcher/compare/0abd405..d6d573d):
  d6d573d  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/matcher#250)

mime (https://github.com/dart-lang/mime/compare/fd7010b..11fec7d):
  11fec7d  2024-07-01  Kevin Moore  blast_repo fixes (dart-archive/mime#131)
  ffdcde3  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.6 to 4.1.7 (dart-archive/mime#129)

mockito (https://github.com/dart-lang/mockito/compare/9deddcf..a7fdf71):
  a7fdf71  2024-06-27  Sam Rawlins  Use curly braces in if statement, in accordance with upcoming lint rule change.

path (https://github.com/dart-lang/path/compare/04807b6..e969f42):
  e969f42  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/path#169)

pool (https://github.com/dart-lang/pool/compare/832c5ab..924fb04):
  924fb04  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (dart-lang/pool#90)

pub_semver (https://github.com/dart-lang/pub_semver/compare/dfcad38..d9e5ee6):
  d9e5ee6  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/pub_semver#106)

shelf (https://github.com/dart-lang/shelf/compare/2536c15..9f2dffe):
  9f2dffe  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/shelf#438)

source_map_stack_trace (https://github.com/dart-lang/source_map_stack_trace/compare/96a8213..741b6ce):
  741b6ce  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/source_map_stack_trace#56)

sse (https://github.com/dart-lang/sse/compare/7dcde16..52d042f):
  52d042f  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/sse#112)

stack_trace (https://github.com/dart-lang/stack_trace/compare/ab09060..4fd3e2a):
  4fd3e2a  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/stack_trace#156)

stream_channel (https://github.com/dart-lang/stream_channel/compare/dc620d2..28a6533):
  28a6533  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/stream_channel#109)

string_scanner (https://github.com/dart-lang/string_scanner/compare/e1cab8f..0de03b5):
  0de03b5  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/string_scanner#77)

term_glyph (https://github.com/dart-lang/term_glyph/compare/6c2a977..38a158f):
  38a158f  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/term_glyph#55)

test (https://github.com/dart-lang/test/compare/329c6df..3256c23):
  3256c23c  2024-07-01  dependabot[bot]  Bump the github-actions group with 3 updates (dart-lang/test#2247)
  4da62ac6  2024-06-27  Nate Bosch  Add package config URI into VM test entrypoints (dart-lang/test#2245)

test_descriptor (https://github.com/dart-lang/test_descriptor/compare/2f19400..90743bc):
  90743bc  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/test_descriptor#69)

test_process (https://github.com/dart-lang/test_process/compare/a7ca20b..6223572):
  6223572  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/test_process#61)

test_reflective_loader (https://github.com/dart-lang/test_reflective_loader/compare/816942e..6e64886):
  6e64886  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/test_reflective_loader#64)

tools (https://github.com/dart-lang/tools/compare/4c60686..43a8582):
  43a8582  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/tools#283)

typed_data (https://github.com/dart-lang/typed_data/compare/8529929..365468a):
  365468a  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (dart-lang/typed_data#91)

vector_math (https://github.com/google/vector_math.dart/compare/a4304d1..2cfbe2c):
  2cfbe2c  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (google/vector_math.dart#329)

watcher (https://github.com/dart-lang/watcher/compare/f312f1f..0484625):
  0484625  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/watcher#170)

web (https://github.com/dart-lang/web/compare/2ae509e..e4c4d81):
  e4c4d81  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/web#269)

web_socket_channel (https://github.com/dart-lang/web_socket_channel/compare/bf69990..8e95ea7):
  8e95ea7  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/web_socket_channel#376)

webdriver (https://github.com/google/webdriver.dart/compare/f85779e..718e4c3):
  718e4c3  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.7 (google/webdriver.dart#301)

webkit_inspection_protocol (https://github.com/google/webkit_inspection_protocol.dart/compare/5740cc9..32fffa5):
  32fffa5  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart from 1.6.2 to 1.6.5 (google/webkit_inspection_protocol.dart#126)
  73ab344  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.7 (google/webkit_inspection_protocol.dart#125)

yaml (https://github.com/dart-lang/yaml/compare/4cf24ca..30fd9e0):
  30fd9e0  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (dart-lang/yaml#168)

yaml_edit (https://github.com/dart-lang/yaml_edit/compare/ad3292c..57a28da):
  57a28da  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/yaml_edit#92)

Change-Id: Ifaff2db977be0b38b631e8a177bbff47c3d24c12
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373880
Reviewed-by: Kevin Moore <kevmoo@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
kenzieschmoll added a commit to flutter/flutter that referenced this pull request Jul 9, 2024
@natebosch
Copy link
Member Author

I noticed that this is only likely to work for dart test. If a VM test is run with flutter test I think the package config we pick up here will be the one for the flutter repo, not the user package.

@kenzieschmoll - when you tested to confirm behavior for the VM tests, did you use dart test only? Or did you try VM tests with flutter test in addition to the platform you modified?

@kenzieschmoll
Copy link

This works with flutter test too.

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

Successfully merging this pull request may close these issues.

3 participants