test: run tests on multiple node versions by dymart · Pull Request #22420 · angular/angular-cli

dymart · 2021-12-22T19:17:41Z

No description provided.

jbedard · 2022-05-10T20:44:26Z

Note the current test failure seems to be because the median test run is faster then expected? See poll_spec.ts

alan-agius4 · 2022-06-08T19:42:18Z

Is this something that you still want to land?

jbedard · 2022-06-08T20:47:52Z

Yeah we'd still like to do this one. I had I think all but one test green but have been working on #23074 instead lately.

jbedard · 2022-06-24T20:38:20Z

@alan-agius4 there is one TODO left here but I think this is worth landing as-is for now. WDYT?

alan-agius4 · 2022-06-27T08:38:40Z

+# TODO: test on multiple node versions
+# Some tests use specific ports and can not be run concurrently
+# Some tests fail for unknown reasons when run on multiple platforms


These tests are probably the most important that should be ran on multiple platforms.

Do you have examples of the failures?

Probably we can investigate this in the near future.

One reason migth be the devservers all using port 4200 which we should be able to configure to be random. I'll look into that more if this test is important.

That shouldn’t be hard to do as the CLI does support port 0.

I can take a look at these maybe in a week.

I am happy to get the setup in before that. Unless @clydin has any reservations.

I'm going to work on this and push to this PR but you can merge the first commit now if you'd like

jbedard · 2022-06-29T17:32:33Z

 # Architect resolves output files from data files, and this isn't possible without runfile support.
 test --enable_runfiles
+
+build --experimental_remote_merkle_tree_cache


This caches the analysis of bazel actions better. Because we have exports_directories_only = False (which ts_library requires?) every single file in the node_moduels gets traversed instead of only the package folders which creates a giant tree. This is also the reason for increasing the circleci resource_class.

jbedard · 2022-06-29T19:01:44Z

The flakyness of these tests is very odd, and I think a lot of it has nothing to do with this change. Things like @angular-devkit/core not being found errors 🤔

alan-agius4 · 2022-06-30T05:37:19Z

The flakyness of these tests is very odd, and I think a lot of it has nothing to do with this change. Things like @angular-devkit/core not being found errors 🤔

Strange as I haven’t seen any similar flakes on the main branch recently.

jbedard · 2022-07-04T18:22:56Z

        exclude_prefixes = [
            "packages",  # Exclude compiled outputs of dependent packages
        ],
+        allow_overwrites = True,


This needed to be added due to an update to aspect_bazel_lib that someone else must have done. I don't fully understand why the build didn't fail until this PR though?

I've reverted this for now to make this PR the absolute minimum number of changes.

jbedard · 2022-07-13T20:49:41Z

@alan-agius4 we'd like to merge this as-is without actually enabling the multiple versions and just setting up the toolchains + BUILDs.

It seems like these tests are very flaky and doubling the number of them makes it fail what seems like 100% of the time. Specifically the packages/angular_devkit/build_angular tests which are put under the LARGE_SPECS variable name, the rest actually seem ok if we want to enable the rest (14/15 of them). This test even fails sometimes with no changes at all (like the PR currently is - only adding the toolchains config).

alan-agius4 · 2022-07-27T08:00:31Z

@jbedard can you please rebase?
As I am unable to do that for you as edits by maintainers is not allowed for this PR.

alan-agius4 · 2022-08-08T09:17:17Z

Since this change went in, test flakiness have increased.

One of the main errors is

java.lang.OutOfMemoryError: Java heap space

jbedard · 2022-08-08T17:05:53Z

@alan-agius4 can we increase the circleci resource_class?

jbedard · 2022-08-08T18:30:28Z

Or it might actually be better as a separate circleci step per node version? Then we're doubling the number of steps instead of number of concurrent tests. That is probably required for the e2e tests so might be a good prefactor that we can do first...

alan-agius4 · 2022-08-08T19:07:40Z

We have discussed this during our meeting today and for the time being until we find out the cause of the increase in memory usage. We will like to disable testing on multiple node versions.

The main reasons for this is that we don’t have many targets which justice usage of 14Gb memory considering we also use RBE. An increase the resource class would also double the credit usage if we go with 2 X-large.

I am curious to know which additional “steps” you thing we can share.

jbedard · 2022-08-08T19:38:31Z

The increase in memory usage is from doubling the number of tests running concurrently in the test circleci job.

Today we're running the different node versions concurrently in one circleci step within the test job. We could do the same as the e2e-tests and use matrix params to create a separate job per node version, or create a separate step per node version within the test job so they run sequentially within a single job.

alan-agius4 · 2022-08-08T19:56:59Z

The increase in memory usage is from doubling the number of tests running concurrently in the test circleci job.

But we are using RBE so they are not actually run on circle. Also I wouldn’t expect 14GB of memory to be used just to run these tests considering we don’t have many targets.

jbedard · 2022-08-08T20:10:03Z

But we are using RBE so they are not actually run on circle.

Right... you're sure bazel reporting those means it is the circleci machine and not the remote one?

jbedard · 2022-08-08T20:49:02Z

But we are using RBE so they are not actually run on circle.

Looking at the bazelrc... isn't that only configuring bazel build (not test) to run remotely?

alan-agius4 · 2022-08-08T21:04:17Z

test inherits from build.

https://docs.bazel.build/versions/main/guide.html#option-defaults

jbedard · 2022-08-08T21:34:26Z

If it is running remotely why would browser-tools/install-chrome be required? Is there any way to verify it's actually running remote? I'm not too familiar with circleci or remote exec...

alan-agius4 · 2022-08-09T10:23:34Z

It does appear that there are some tests that run locally.

INFO: 679 processes: 215 remote cache hit, 288 internal, 40 local, 4 processwrapper-sandbox, 85 remote, 47 worker.

jbedard · 2022-08-09T17:17:00Z

Right, there are a few tagged as no-remote I think due to protractor/webdriver issues:

angular-cli/packages/angular_devkit/build_angular/BUILD.bazel

Lines 298 to 299 in 9171543

    
           # NB: does not run on rbe because webdriver manager uses an absolute path to chromedriver 
        
           "tags": ["no-remote-exec"],

(and another one down a bit)

angular-automatic-lock-bot · 2022-09-09T08:04:36Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

dymart changed the title ~~test(@angular/cli,@angular/pwa,@angular-devkit/architect,@angular-dev…~~ test: add multiple node versions for each test Dec 23, 2021

jbedard changed the title ~~test: add multiple node versions for each test~~ test: run tests on multiple node versions Apr 29, 2022

jbedard reviewed Apr 29, 2022

View reviewed changes

Comment thread packages/angular_devkit/build_angular/BUILD.bazel Outdated

jbedard requested a review from josephperrott April 29, 2022 17:39

jbedard requested review from alan-agius4 and kormide May 10, 2022 20:42

jbedard requested a review from clydin June 24, 2022 20:38

alan-agius4 reviewed Jun 27, 2022

View reviewed changes

Comment thread packages/angular_devkit/build_angular/BUILD.bazel Outdated

alan-agius4 added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: minor This PR is targeted for the next minor release labels Jun 29, 2022

jbedard reviewed Jun 29, 2022

View reviewed changes

alan-agius4 reviewed Jun 30, 2022

View reviewed changes

Comment thread packages/angular_devkit/build_angular/src/builders/dev-server/tests/options/port_spec.ts Outdated

jbedard reviewed Jul 4, 2022

View reviewed changes

jbedard requested a review from alan-agius4 July 18, 2022 17:15

alan-agius4 reviewed Jul 27, 2022

View reviewed changes

Comment thread tools/toolchain_info.bzl Outdated

alan-agius4 approved these changes Jul 27, 2022

View reviewed changes

jbedard and others added 2 commits August 2, 2022 11:07

build: enable bazel remote_merkle_tree_cache

cd69988

test: run tests on multiple node versions

9635dc5

alan-agius4 approved these changes Aug 2, 2022

View reviewed changes

alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Aug 2, 2022

alan-agius4 added target: patch This PR is targeted for the next patch release and removed target: minor This PR is targeted for the next minor release labels Aug 2, 2022

dgp1130 merged commit db5c1fc into angular:main Aug 3, 2022

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 9, 2022

Uh oh!

Conversation

dymart commented Dec 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jbedard commented May 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alan-agius4 commented Jun 8, 2022

Uh oh!

jbedard commented Jun 8, 2022

Uh oh!

jbedard commented Jun 24, 2022

Uh oh!

alan-agius4 Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

jbedard Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

alan-agius4 Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

alan-agius4 Jun 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbedard Jun 27, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jbedard Jun 29, 2022

Choose a reason for hiding this comment

Uh oh!

jbedard commented Jun 29, 2022

Uh oh!

alan-agius4 commented Jun 30, 2022

Uh oh!

Uh oh!

jbedard Jul 4, 2022

Choose a reason for hiding this comment

Uh oh!

jbedard Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

jbedard commented Jul 13, 2022

Uh oh!

Uh oh!

alan-agius4 commented Jul 27, 2022

Uh oh!

alan-agius4 commented Aug 8, 2022

Uh oh!

jbedard commented Aug 8, 2022

Uh oh!

jbedard commented Aug 8, 2022

Uh oh!

alan-agius4 commented Aug 8, 2022

Uh oh!

jbedard commented Aug 8, 2022

Uh oh!

alan-agius4 commented Aug 8, 2022

Uh oh!

jbedard commented Aug 8, 2022

Uh oh!

jbedard commented Aug 8, 2022

Uh oh!

alan-agius4 commented Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbedard commented Aug 8, 2022

Uh oh!

alan-agius4 commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbedard commented Aug 9, 2022

Uh oh!

angular-automatic-lock-bot Bot commented Sep 9, 2022

dymart commented Dec 22, 2021 •

edited

Loading

jbedard commented May 10, 2022 •

edited

Loading

alan-agius4 Jun 27, 2022 •

edited

Loading

alan-agius4 commented Aug 8, 2022 •

edited

Loading

alan-agius4 commented Aug 9, 2022 •

edited

Loading