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

Tests failing on current master #11062

Closed
aladdin-add opened this issue Nov 7, 2018 · 9 comments
Closed

Tests failing on current master #11062

aladdin-add opened this issue Nov 7, 2018 · 9 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly build This change relates to ESLint's build process

Comments

@aladdin-add
Copy link
Member

aladdin-add commented Nov 7, 2018

see https://ci.appveyor.com/project/nzakas/eslint/builds/20097976

@aladdin-add aladdin-add added bug ESLint is working incorrectly build This change relates to ESLint's build process accepted There is consensus among the team that this change meets the criteria for inclusion labels Nov 7, 2018
@nzakas
Copy link
Member

nzakas commented Nov 7, 2018

Confirmed. This is blocking PRs right now.

@not-an-aardvark
Copy link
Member

not-an-aardvark commented Nov 7, 2018

Based on the test failures this might have been caused by the recent release of flat-cache. However, I don't have time to look into it in more detail at the moment.

@nzakas
Copy link
Member

nzakas commented Nov 7, 2018

I'll try to take a look in the few minutes of remaining energy I have.

@nzakas
Copy link
Member

nzakas commented Nov 7, 2018

Related: #11052

@nzakas
Copy link
Member

nzakas commented Nov 7, 2018

It looks like the contents of .eslintcache is now an array instead of an object, and all of the tests are failing because they are expecting the cache file to contain an object with keys that are the filenames.

I'm not sure that this has actually changed the functionality of CLIEngine, because we are reading from and writing to the cache using the API, whereas in the tests we are reading the cache file directly.

However, if the cache file format has changed, that will cause a problem for people who have an existing cache file and then upgrade or reinstall ESLint.

@royriojas any thoughts?

@royriojas
Copy link
Contributor

royriojas commented Nov 7, 2018

hi @nzakas,

I recently released a new version of the flat-cache as a patch, which is in time used by file-entry-cache, but the unit tests passed in both flat-cache and file-entry-cache that's why I believe it was safe.

The newer flat-cache is now using flatted as it was changed in this PR. jaredwray/flat-cache#23

Will take some time to review if that might be interpreting/serializing differently the cache, as I said, I wouldn't expect it to happen as tests passed. But it might be we're lacking one test to protect against cases like this.

@royriojas
Copy link
Contributor

royriojas commented Nov 7, 2018

I've reverted the upgrade of circular-json to flatted :( and published it as flat-cache@1.3.2

Will add a test for the eslint case before attempting to upgrade again

I verified master in eslint and tests pass just fine.

@nzakas
Copy link
Member

nzakas commented Nov 8, 2018

Thanks @royriojas looks like we are in the clear now.

@aladdin-add
Copy link
Member Author

closing, as the issue has been fixed.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators May 9, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly build This change relates to ESLint's build process
Projects
None yet
Development

No branches or pull requests

4 participants