Skip to content

[wasm][tests] Fix, and simplify sample building on CI#63429

Merged
radical merged 7 commits intodotnet:mainfrom
radical:sample-fix
Jan 8, 2022
Merged

[wasm][tests] Fix, and simplify sample building on CI#63429
radical merged 7 commits intodotnet:mainfrom
radical:sample-fix

Conversation

@radical
Copy link
Member

@radical radical commented Jan 6, 2022

  1. Instead of hardcoding specific command lines for wasm samples in sendtohelixhelp.proj, generate a run script, which allows moving the execution commands to the respective projects.
  2. this enables running the library tests additionally for v8, and browser, in case of runtime-manual. This was recently changed to run only for nodejs
  3. And avoids running samples on debugger tests build

@ghost ghost assigned radical Jan 6, 2022
@ghost ghost added the area-Build-mono label Jan 6, 2022
@radical radical added arch-wasm WebAssembly architecture and removed area-Build-mono labels Jan 6, 2022
@ghost
Copy link

ghost commented Jan 6, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: radical
Assignees: radical
Labels:

arch-wasm

Milestone: -

@radical
Copy link
Member Author

radical commented Jan 6, 2022

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Jan 6, 2022

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Jan 6, 2022

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Jan 6, 2022

/azp run runtime-manual

@radical radical marked this pull request as ready for review January 6, 2022 21:37
@radical radical requested a review from marek-safar as a code owner January 6, 2022 21:37
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical radical marked this pull request as draft January 6, 2022 21:39
@radical radical marked this pull request as ready for review January 6, 2022 21:52
@radical
Copy link
Member Author

radical commented Jan 6, 2022

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Jan 6, 2022

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Jan 7, 2022

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines failed to run 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Jan 7, 2022

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical
Copy link
Member Author

radical commented Jan 7, 2022

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

creator: dotnet-bot
testRunNamePrefixSuffix: Mono_$(_BuildConfig)
scenarios:
- normal
Copy link
Member

Choose a reason for hiding this comment

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

Comment on the section said NodeJs, should we fix it ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a follow up PR, I'll remove the comment in that.

eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_mono.containsChange'], true),
eq(dependencies.evaluate_paths.outputs['SetPathVars_installer.containsChange'], true),
eq(variables['isManualOrIsNotPR'], true),
Copy link
Member

Choose a reason for hiding this comment

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

The same conditions you removed are duplicated in many other places of this file. Should we remove them too ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@steveisok would you prefer that? Or should I restore the conditions, and just add the manual one?

Copy link
Member

Choose a reason for hiding this comment

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

I think that's ok.

Copy link
Member

@pavelsavara pavelsavara left a comment

Choose a reason for hiding this comment

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

LGTM, my comments are more questions than feedback.

@radical
Copy link
Member Author

radical commented Jan 8, 2022

Failures are unrelated.

@radical radical merged commit cc7cd63 into dotnet:main Jan 8, 2022
@radical radical deleted the sample-fix branch January 8, 2022 04:45
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-Build-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants