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

CI does not compile test apps #7683

Open
pmconne opened this issue Feb 10, 2025 · 1 comment
Open

CI does not compile test apps #7683

pmconne opened this issue Feb 10, 2025 · 1 comment
Labels
bug Something isn't working buildology Issues related to process, tooling or CI/CD pipelines

Comments

@pmconne
Copy link
Member

pmconne commented Feb 10, 2025

Describe the bug
The build scripts for apps like display-test-app and display-performance-test-app use vite build, not tsc. The latter reports errors that the former does not. Those errors then surface at run-time. This makes it possible to merge to master changes that do not compile, that can only be discovered when/if someone encounters the conditions which would cause the non-compiling code to execute.

To Reproduce
Steps to reproduce the behavior:

  1. Pull itwinjs-core master branch as of the time of this issue.
  2. rush build --to display-test-app.
  3. Observe no compilation errors.
  4. cd test-apps/display-test-app
  5. npm run build:frontend
  6. Observe compilation errors below. Note these various errors were introduced over the course of multiple separate PRs, waiting for someone to discover them.
npm run build:frontend

> [email protected] build:frontend
> tsc

src/frontend/DisplayScale.ts:13:10 - error TS2416: Property 'getModelDisplayTransform' in type 'DisplayScaleTransformProvider' is not assignable to the same property in base type 'ModelDisplayTransformProvider'.
  Type '(modelId: string) => Transform | undefined' is not assignable to type '(modelId: string) => ModelDisplayTransform | undefined'.
    Type 'Transform | undefined' is not assignable to type 'ModelDisplayTransform | undefined'.
      Property 'transform' is missing in type 'Transform' but required in type 'ModelDisplayTransform'.

13   public getModelDisplayTransform(modelId: string): Transform | undefined {
            ~~~~~~~~~~~~~~~~~~~~~~~~

  ../../core/frontend/lib/cjs/ViewState.d.ts:43:5
    43     transform: Transform;
           ~~~~~~~~~
    'transform' is declared here.

src/frontend/DisplayScale.ts:62:41 - error TS2345: Argument of type 'DisplayScaleTransformProvider' is not assignable to parameter of type 'ModelDisplayTransformProvider'.
  The types returned by 'getModelDisplayTransform(...)' are incompatible between these types.
    Type 'Transform | undefined' is not assignable to type 'ModelDisplayTransform | undefined'.
      Property 'transform' is missing in type 'Transform' but required in type 'ModelDisplayTransform'.

62     vp.setModelDisplayTransformProvider(tp);
                                           ~~

  ../../core/frontend/lib/cjs/ViewState.d.ts:43:5
    43     transform: Transform;
           ~~~~~~~~~
    'transform' is declared here.

src/frontend/DisplayTestApp.ts:189:28 - error TS2339: Property 'enableDiagnostics' does not exist on type 'RenderSystem'.

189     IModelApp.renderSystem.enableDiagnostics();
                               ~~~~~~~~~~~~~~~~~

src/frontend/DisplayTestApp.ts:210:30 - error TS2339: Property 'enableDiagnostics' does not exist on type 'RenderSystem'.

210       IModelApp.renderSystem.enableDiagnostics();
                                 ~~~~~~~~~~~~~~~~~

src/frontend/OutputShadersTool.ts:5:10 - error TS2305: Module '"@itwin/core-frontend"' has no exported member 'DebugShaderFile'.

5 import { DebugShaderFile, IModelApp, NotifyMessageDetails, OutputMessagePriority, Tool } from "@itwin/core-frontend";
           ~~~~~~~~~~~~~~~

src/frontend/TileContentTool.ts:18:44 - error TS2304: Cannot find name 'IModelTileTree'.

18   public override async run(args?: { tree: IModelTileTree, contentId: string }) {
                                              ~~~~~~~~~~~~~~

src/frontend/TileSizeRecorder.ts:8:21 - error TS2305: Module '"@itwin/core-frontend"' has no exported member 'IModelTileTree'.

8 import { IModelApp, IModelTileTree, TileAdmin, Tool } from "@itwin/core-frontend";
                      ~~~~~~~~~~~~~~

src/frontend/TiledGraphics.ts:60:10 - error TS2551: Property 'forEachModelTreeRef' does not exist on type 'ViewState'. Did you mean 'forEachTileTreeRef'?

60     view.forEachModelTreeRef((ref) => {
            ~~~~~~~~~~~~~~~~~~~

  ../../core/frontend/lib/cjs/ViewState.d.ts:289:5
    289     forEachTileTreeRef(func: (treeRef: TileTreeReference) => void): void;
            ~~~~~~~~~~~~~~~~~~
    'forEachTileTreeRef' is declared here.

src/frontend/TiledGraphics.ts:60:31 - error TS7006: Parameter 'ref' implicitly has an 'any' type.

60     view.forEachModelTreeRef((ref) => {
                                 ~~~


Found 9 errors in 6 files.

Errors  Files
     2  src/frontend/DisplayScale.ts:13
     2  src/frontend/DisplayTestApp.ts:189
     1  src/frontend/OutputShadersTool.ts:5
     1  src/frontend/TileContentTool.ts:18
     1  src/frontend/TileSizeRecorder.ts:8
     2  src/frontend/TiledGraphics.ts:60

Expected behavior
CI should fail if compilation errors exist.

Ideally, rush build will notify me of compilation errors locally as well, it is extremely annoying to edit code, run the app, and then discover a compilation error. The whole point of using a compiled language is to discover errors before running the code.

@pmconne pmconne added bug Something isn't working buildology Issues related to process, tooling or CI/CD pipelines labels Feb 10, 2025
@tcobbs-bentley
Copy link
Member

I would like to add here that the Android and iOS steps in the CI both compile and run display-test-app in a simulator/emulator. However, all they do is download a specific model from the hub, open that model, and wait for it to render the first time. So presumably the functionality that causes problems doesn't get exercised by that sequence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working buildology Issues related to process, tooling or CI/CD pipelines
Projects
None yet
Development

No branches or pull requests

2 participants