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.

Sign up for GitHub

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

Jump to bottom

TS-33912 Support For Profiling Gwt Applications #43

Merged
merged 16 commits into from
Aug 16, 2023
Merged

TS-33912 Support For Profiling Gwt Applications #43

merged 16 commits into from
Aug 16, 2023

Conversation

stahlbauer
Copy link
Collaborator

Addresses issue TS-33912

  • No findings in Teamscale (c.f. vote of the Teamscale Bot). Flag irrelevant findings as tolerated or false positives. If you feel that the Teamscale config needs adjustment, please state so in a comment and discuss this with your reviewer.
  • Changes are tested adequately
  • Profilers's README.md and the Teamscale documentation are updated in case of user-visible changes
  • CHANGELOG.md of the affected packages are updated
  • Present new features in N&N

AnonymFx
AnonymFx requested changes Aug 10, 2023
View reviewed changes
packages/teamscale-javascript-instrumenter/Promise] Outdated Show resolved Hide resolved
packages/teamscale-javascript-instrumenter/bench-perf-stats.dat Outdated Show resolved Hide resolved
packages/teamscale-javascript-instrumenter/src/instrumenter/Task.ts Outdated Show resolved Hide resolved
packages/teamscale-javascript-instrumenter/src/instrumenter/WebToolkit.ts Outdated Show resolved Hide resolved
packages/teamscale-javascript-instrumenter/src/instrumenter/WebToolkit.ts Show resolved Hide resolved
packages/teamscale-javascript-instrumenter/src/instrumenter/Instrumenter.ts Outdated Show resolved Hide resolved
packages/teamscale-javascript-instrumenter/test/unit/FileSystem.test.ts
import { findSubFolders } from '../../src/instrumenter/FileSystem';

test('Test identification of sub folders with GWT source maps', () => {
const folders = findSubFolders('../../test/casestudies/gwt-showcase-js/', 'symbolMaps');
Copy link
Contributor

Choose a reason for hiding this comment

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

gwt-showcase-js doesn't exist anymore (removed with the latest commit) => All new tests (incl. the ones in WebToolkit.test.ts) fail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the pointer. I have added a reduced version of the case study.

@stahlbauer
Copy link
Collaborator Author

Currently we do not have an automated e2e test for the GWT instrumentation. I would address this in a maintenance ticket in a next step. Likely by deploying one of the GWT sample applications in a local Jetty server and collecting coverage from it.

@stahlbauer
Copy link
Collaborator Author

I have looked into the Teamscale findings. These are tollerable for now. However, I would move the source map logic to a separate module after we decide to keep the GWT support.

@stahlbauer
Copy link
Collaborator Author

If possible, I would merge the functionality to the main branch to avoid having maintaining several branches in parallel. Furthermore, some of the adjustments made will also benefit the overall JS profiler performance and architecture.

@stahlbauer
Copy link
Collaborator Author

I have created a maintenance ticket for adding GWT system tests: TS-33912

@stahlbauer stahlbauer merged commit b45bcf5 into master Aug 16, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnonymFx AnonymFx AnonymFx requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@stahlbauer @AnonymFx