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

docs: explanation on how to deal with tests failing on windows #575

Merged
merged 6 commits into from
Jun 14, 2023
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,17 @@ A few advantages of this solution:

## Develop

### Windows
Copy link
Member

Choose a reason for hiding this comment

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

I would move this section below the already existing list of steps to do during development, and then the line about the windows specific scenario you place this section in the end.

The reason is that this list of steps is not only related to windows. It is only the special case about the file ending 🙂


1. Write code and tests.
1. Make sure all tests pass `npm test`
1. Make sure code is well formatted and secure `npm run lint`
2. Make sure all tests pass `npm test`

If in-case some tests still fails randomly during local development, the reason to it might be when you develop something in the parser and even though you did not make any changes affects tests, and the tests are failing then you can run:
Copy link
Member

Choose a reason for hiding this comment

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

Since this is only relevant for windows systems, I think we should highlight that somehow here, what do you think @Ruchip16🤔?

Maybe it makes sense to add a sub section such as ### Windows environments and place this information there? 🤔

cc @alequetzalli in case you have time and have an opinion 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes we can add that also then we need to add sub-sections for Mac or Linux users as well then right?

Copy link
Member

Choose a reason for hiding this comment

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

Not unless you have something specific you want to add there 🙂 To me a section only makes sense if there is something to highlight, i.e. in this case windows environment might act weird in some cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed, would do the changes in the PR

Copy link
Member

Choose a reason for hiding this comment

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

@jonaslagoni Totally, if it's just for Windows OS, let's def add a ### Windows title.

(I don't think you need to write the word 'environment' too, it's pretty clear to just say Windows.)

Copy link
Member

Choose a reason for hiding this comment

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

We need to change this sentence I think: the reason to it might be when you develop something in the parser and even though you did not make any changes affects tests

To me it is not clear what exactly the reason is. Maybe something along the lines of:

Suggested change
If in-case some tests still fails randomly during local development, the reason to it might be when you develop something in the parser and even though you did not make any changes affects tests, and the tests are failing then you can run:
For Windows environments, some tests might still fail randomly during local development even when you made no changes to the tests. The reason for this from file endings are different than expected and this comes from Git defaulting to an unexpected file ending. If you encounter this issue you can run the following commands to set Git to use the expected one:

What do you think @Ruchip16 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yess absolutely the later one gives more clarity than its prior, I would do the changes

```
git config --global core.autocrlf false
git config --global core.eol lf
```
3. Make sure code is well formatted and secure `npm run lint`

Release regenerates API documentation and browser bundle, so you do not have to regenerate it manually with `npm run docs` and `npm run prepublishOnly`.

Expand Down