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: watch option snapshot #2679

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

test: watch option snapshot #2679

wants to merge 2 commits into from

Conversation

anshumanv
Copy link
Member

What kind of change does this PR introduce?
test

Did you add tests for your changes?
it is test

If relevant, did you update the documentation?
NA

Summary
snapshot testing for watch option/command

Does this PR introduce a breaking change?
no

Other information

@anshumanv anshumanv requested a review from a team as a code owner April 29, 2021 21:05
.replace(/Hash: .*/gm, 'Hash: <hash>')
.replace(/Time: .*/gm, 'Time: <compile time>')
.replace(/Built at: .*/gm, 'Built at: <built time>');
};
Copy link
Member

Choose a reason for hiding this comment

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

Put normalizeCompileTime in normalizeV4Output, ideally we should have two function normalizeStatsV4 and normalizeStatsV5, v5 default output are simple so we should not have big problems here, If you need I can implement these functions so you will be able to continue adding snapshots and improve normalizeStats function using regexps

Copy link
Member Author

Choose a reason for hiding this comment

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

yep help appreciated, also something wrong with v4 snapshot, thanks!

@@ -247,6 +247,17 @@ const normalizeError = (output) => {
return output.replace(/SyntaxError: .+/, 'SyntaxError: <error-message>').replace(/\s+at .+(}|\)|\d)/gs, '\n at stack');
};

const normalizeCompileTime = (output) => {
return output.replace(/in \d+ ms/gm, 'in <compile time> ms').replace();
Copy link
Member

Choose a reason for hiding this comment

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

explain your regexes :)

Copy link
Member Author

Choose a reason for hiding this comment

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

yep better to add an explanation and some examples of what's being replaced

@alexander-akait
Copy link
Member

Incident with GitHub Actions https://www.githubstatus.com/, the universe does not want to let me finish at all webpack-dev-server

@anshumanv
Copy link
Member Author

Actions are always painful 😞

@alexander-akait
Copy link
Member

@anshumanv Can you rebase, I want to finish, so we will continue migrate on snapshots

@anshumanv
Copy link
Member Author

on it

@anshumanv
Copy link
Member Author

@alexander-akait done :)

@alexander-akait
Copy link
Member

We need do some improvements for runWatch and runAndGetProcess, I will do it tomorrow and we will finish it, you took the hardest modes 😄

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

could you rebase?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants