Fixing path setup in the windows platform scripts. #594
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mcottontensor
added
auto-backport
github-actions bot
pushed a commit
that referenced
this pull request
Apr 11, 2025
(cherry picked from commit 152abd6)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
mcottontensor
added a commit
that referenced
this pull request
Apr 11, 2025
mcottontensor
added a commit
that referenced
this pull request
May 1, 2025
mcottontensor
added a commit
that referenced
this pull request
May 29, 2025
… support (#647) * Add 4.27 support back for latency testing * Save webrtc stats to csv and auto download * Deep copy stats object * Update record parsing logic to handle missing field when generating the csv * Update session test to handle additional latency stats * Fixing path setup in the windows platform scripts. (#594) (#595) (cherry picked from commit 152abd6) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> (cherry picked from commit 1dedc87) * Fixing more path issues in platform_scripts (#604) * feat: Adding script tests where the scripts are run multiple times. This tests that the scripts still work when things have already been installed. * fix: Fixing typo in github action. * gimme a break. * Adding mutliple script runs for other OSs * Testing node on the runner * Trying to figure out what node is doing what. * Trying to change the way node version is detected. * Messy * The additions to the script really didn't catch anything. Could be worth revisiting how we handle the local node install. * Removing debug print. (cherry picked from commit 2321fa9) * refactor: Better handling of node in Linux scripts. (#607) Updated the way the bash script sets up the environment for node. Node will only be installed if a version meeting the minimum requirements is not found. The new environment should result in a more robust environment setup. (cherry picked from commit f27bace) * Fixing a small issue with how path was setup previously. (#611) If the node install was installed locally previously the script would not pick that up and try to install again. It would fail because the directory already exists and then the path would not get properly setup for the local install of node. This change sets PATH to point to the local install anyway. If it doesn't exist it doesn't matter, if it does exist we use the local install. (cherry picked from commit 00aaa52) * Added disable button when session test is running * Added new flag ?LatencyCSV to enable the session test and its UI * Added changeset for 4.27 support and session test feature --------- Co-authored-by: Will Belcher <william.belcher@xa.epicgames.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com>
github-actions bot
added a commit
that referenced
this pull request
May 29, 2025
… support (#647) * Add 4.27 support back for latency testing * Save webrtc stats to csv and auto download * Deep copy stats object * Update record parsing logic to handle missing field when generating the csv * Update session test to handle additional latency stats * Fixing path setup in the windows platform scripts. (#594) (#595) (cherry picked from commit 152abd6) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> (cherry picked from commit 1dedc87) * Fixing more path issues in platform_scripts (#604) * feat: Adding script tests where the scripts are run multiple times. This tests that the scripts still work when things have already been installed. * fix: Fixing typo in github action. * gimme a break. * Adding mutliple script runs for other OSs * Testing node on the runner * Trying to figure out what node is doing what. * Trying to change the way node version is detected. * Messy * The additions to the script really didn't catch anything. Could be worth revisiting how we handle the local node install. * Removing debug print. (cherry picked from commit 2321fa9) * refactor: Better handling of node in Linux scripts. (#607) Updated the way the bash script sets up the environment for node. Node will only be installed if a version meeting the minimum requirements is not found. The new environment should result in a more robust environment setup. (cherry picked from commit f27bace) * Fixing a small issue with how path was setup previously. (#611) If the node install was installed locally previously the script would not pick that up and try to install again. It would fail because the directory already exists and then the path would not get properly setup for the local install of node. This change sets PATH to point to the local install anyway. If it doesn't exist it doesn't matter, if it does exist we use the local install. (cherry picked from commit 00aaa52) * Added disable button when session test is running * Added new flag ?LatencyCSV to enable the session test and its UI * Added changeset for 4.27 support and session test feature --------- Co-authored-by: Will Belcher <william.belcher@xa.epicgames.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> (cherry picked from commit c61fd7b)
mcottontensor
added a commit
that referenced
this pull request
May 29, 2025
… support (#647) (#651) * Add 4.27 support back for latency testing * Save webrtc stats to csv and auto download * Deep copy stats object * Update record parsing logic to handle missing field when generating the csv * Update session test to handle additional latency stats * Fixing path setup in the windows platform scripts. (#594) (#595) (cherry picked from commit 152abd6) (cherry picked from commit 1dedc87) * Fixing more path issues in platform_scripts (#604) * feat: Adding script tests where the scripts are run multiple times. This tests that the scripts still work when things have already been installed. * fix: Fixing typo in github action. * gimme a break. * Adding mutliple script runs for other OSs * Testing node on the runner * Trying to figure out what node is doing what. * Trying to change the way node version is detected. * Messy * The additions to the script really didn't catch anything. Could be worth revisiting how we handle the local node install. * Removing debug print. (cherry picked from commit 2321fa9) * refactor: Better handling of node in Linux scripts. (#607) Updated the way the bash script sets up the environment for node. Node will only be installed if a version meeting the minimum requirements is not found. The new environment should result in a more robust environment setup. (cherry picked from commit f27bace) * Fixing a small issue with how path was setup previously. (#611) If the node install was installed locally previously the script would not pick that up and try to install again. It would fail because the directory already exists and then the path would not get properly setup for the local install of node. This change sets PATH to point to the local install anyway. If it doesn't exist it doesn't matter, if it does exist we use the local install. (cherry picked from commit 00aaa52) * Added disable button when session test is running * Added new flag ?LatencyCSV to enable the session test and its UI * Added changeset for 4.27 support and session test feature --------- (cherry picked from commit c61fd7b) Co-authored-by: Luke Bermingham <1215582+lukehb@users.noreply.github.com> Co-authored-by: Will Belcher <william.belcher@xa.epicgames.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relevant components:
Problem statement:
On fresh windows installs, the platform scripts were giving cryptic failures. Issues with npm cleanup not being able to delete certain paths etc.
Solution
The real problem is that we weren't setting the path variable to the specific node installation we wanted to use. In some instances we were calling the node executable by full path but within the tooling itself the executable was expected to be in the path and so things basically failed in weird ways.