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

test: add tests for ui #241

Merged
merged 10 commits into from
Aug 21, 2024
Merged

test: add tests for ui #241

merged 10 commits into from
Aug 21, 2024

Conversation

AriPerkkio
Copy link
Member

@AriPerkkio AriPerkkio commented Aug 13, 2024

Structure

Test cases are located in test directory.
Each test file has its own chapter, that contains lessons for test cases:

For example Navigation tests:

├── src/content/tutorial
│   └── tests
│       └──── navigation
│           ├── page-one
│           ├── page-three
│           └── page-two
└── test
    └── navigation.test.ts

Or File Tree tests:

├── src/content/tutorial
│   └── tests
│       └── file-tree
│           └── lesson-and-solution
└── test
    └── file-tree.test.ts

Copy link

stackblitz bot commented Aug 13, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@AriPerkkio AriPerkkio force-pushed the test/ui-tests branch 2 times, most recently from 71413a0 to 5671399 Compare August 15, 2024 11:19
@AriPerkkio AriPerkkio marked this pull request as ready for review August 15, 2024 11:39
Comment on lines 32 to 33
// TODO: Figure out why this is flaky
await page.waitForTimeout(1_000);
Copy link
Member Author

@AriPerkkio AriPerkkio Aug 15, 2024

Choose a reason for hiding this comment

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

Couldn't figure out what's the root cause here. Sometimes the content of previous file was shown. Here's picture that shows contents of example.html even though example.js is selected:

image

Looks like the content of example.js was there for a while but then something else changed example.html back on. Syntax highlight is changed to JS. 🤔

@AriPerkkio
Copy link
Member Author

@Nemikolh this is now ready for final review and is no longer blocked by #238.

As this contains couple of fix commits as well, it might be good to merge this without squashing so that the fixes will show up in release notes properly.

@Nemikolh
Copy link
Member

@AriPerkkio I think I can't push to main directly and I do not have access to other merge options.

Note that this would be contrary to our conventions and they've been used to get more details about a commit by looking at the PR number associated with a commit.

In the early days of TutorialKit it was possible but we then decided to always go through PRs.

Do you think you could submit a separate PR that contains all the fixes? Or multiple PRs if you prefer.

@AriPerkkio
Copy link
Member Author

Oh I see, other merge options are completely disabled. It would be good to have squashing enabled by default, but allow rebase when needed.

I'll split this PR and move to fixes into separate PRs. 👍

@AriPerkkio
Copy link
Member Author

Alright, good to go now. Other PRs are in main and this one is up-to-date.

Copy link
Member

@Nemikolh Nemikolh left a comment

Choose a reason for hiding this comment

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

Overall looks good!

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
test/cli/package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
.npmrc Outdated Show resolved Hide resolved
test/ui/src/templates/default/index.mjs Outdated Show resolved Hide resolved
test/ui/src/templates/default/index.mjs Outdated Show resolved Hide resolved
test/ui/src/templates/default/package.json Outdated Show resolved Hide resolved
Copy link
Member

@Nemikolh Nemikolh left a comment

Choose a reason for hiding this comment

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

Looks really good! 👏

Only have a minor comment on ci.yaml and let's get this in! 🎉

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
e2e/README.md Outdated Show resolved Hide resolved
e2e/package.json Outdated Show resolved Hide resolved
e2e/test/utils.ts Show resolved Hide resolved
@Nemikolh Nemikolh merged commit 74e0527 into stackblitz:main Aug 21, 2024
10 checks passed
@AriPerkkio AriPerkkio deleted the test/ui-tests branch August 21, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants