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

LPS-96376 The Liferay.Language.get() operation in javascript requires that the key and call of the operation be on the same line #74563

Closed
wants to merge 1 commit into from

Conversation

inacionery
Copy link

@inacionery inacionery commented Jun 17, 2019

/cc @wincent

… that the key and call of the operation be on the same line
@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering "ci:test:sf" and "ci:test:relevant" for this pull to run Source Formatter and relevant tests.

Comment "ci:test" to run the full PR Tester for this pull.

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutes 20 seconds 451 ms

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: c0c13433600398fed8768f539aa8212978f7409c

Sender Branch:

Branch Name: LPS-96775
Branch GIT ID: a52ce442d7003e35b49d754204ab51c5850011d2

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:relevant - 24 out of 27 jobs passed in 1 hour 19 minutes 29 seconds 600 ms

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: c0c13433600398fed8768f539aa8212978f7409c

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 5e2a3af288c0eee5f221f7d2c9b259f3343ccb24

24 out of 27 jobs PASSED
24 Successful Jobs:
For more details click here.

Failures unique to this pull:

  1. test-portal-acceptance-pullrequest-batch(master)/modules-integration-mysql57-jdk8
    Job Results:

    1025 Tests Passed.
    1 Test Failed.

    1. AXIS_VARIABLE=2,label_exp=!master #301307
      1. ModuleBuildAutoUpgradeTest.testBuildAutoUpgrade
        java.lang.AssertionError: [] expected:<1> but was:<0>
        	at org.junit.Assert.fail(Assert.java:88)
        	at org.junit.Assert.failNotEquals(Assert.java:834)
        	at org.junit.Assert.assertEquals(Assert.java:645)
        	at com.liferay.portal.upgrade.test.BaseBuildAutoUpgradeTestCase._assertAndGetFirstLogRecordMessage(BaseBuildAutoUpgradeTestCase.java:432)
        	at com.liferay.portal.upgrade.test.BaseBuildAutoUpgradeTestCase._updateBundle(BaseBuildAutoUpgradeTestCase.java:499)
        	at com.liferay.portal.upgrade.test.BaseBuildAutoUpgradeTestCase.testBuildAutoUpgrade(BaseBuildAutoUpgradeTestCase.java:167)
        	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        	at java.lang.reflect.Method.invoke(Method.java:498)
        	at com.liferay.arquillian.extension.junit.bridge.server.TestExecutorRunnable$2.evaluate(TestExecutorRunnable.java:218)
        	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
        	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
        	at com.liferay.portal.kernel.test.rule.AbstractTestRule$2.evaluate(AbstractTestRule.java:79)
        	at com.liferay.portal.kernel.test.rule.AbstractTestRule$2.evaluate(AbstractTestRule.java:79)
        	at com.liferay.portal.kernel.test.rule.AbstractTestRule$2.evaluate(AbstractTestRule.java:79)
        	at com.liferay.portal.kernel.test.rule.AbstractTestRule$2.eval...

Failures in common with acceptance upstream results at c0c1343:
  1. test-portal-acceptance-pullrequest-batch(master)/modules-integration-mysql57-jdk8
    Job Results:

    1025 Tests Passed.
    1 Test Failed.

    1. AXIS_VARIABLE=2,label_exp=!master #301307
      1. PortalLogAssertorTest.testScanXMLLog
        junit.framework.AssertionFailedError: 
        Unable to initialize service component
        com.liferay.portal.kernel.exception.OldServiceComponentException: Build namespace BuildAutoUpgradeTest has build number 4 which is newer than 1
        	at com.liferay.portal.service.impl.ServiceComponentLocalServiceImpl.initServiceComponent(ServiceComponentLocalServiceImpl.java:173)
        	at com.liferay.portal.spring.aop.AopMethodInvocationImpl.proceed(AopMethodInvocationImpl.java:50)
        	at com.liferay.portal.spring.transaction.TransactionInterceptor.lambda$invoke$0(TransactionInterceptor.java:64)
        	at com.liferay.portal.spring.transaction.DefaultTransactionExecutor._execute(DefaultTransactionExecutor.java:128)
        	at com.liferay.portal.spring.transaction.DefaultTransactionExecutor.execute(DefaultTransactionExecutor.java:51)
        	at com.liferay.portal.spring.transaction.TransactionInterceptor.invoke(TransactionInterceptor.java:62)
        	at com.liferay.portal.spring.aop.AopMethodInvocationImpl.proceed(AopMethodInvocationImpl.java:57)
        	at com.liferay.portal.spring.aop.AopInvocationHandler.invoke(AopInvocationHandler.java:49)
        	at com.sun.proxy.$Proxy206.initServiceComponent(Unknown Source)
        	at com.liferay.portal.spring.extender.internal.configuration.ServiceConfigurationInitializer._initServiceComponent(ServiceConfigurationInitializer.java:117)
        	at com.liferay.portal.spring.extender.internal.configuration.ServiceConfigurationInitializer.start(ServiceConfigurationInitializer.java:85)
        	at org.apache.felix.dm.InvocationUtil.invo...

@brianchandotcom
Copy link
Owner

Merged. Thx.

@brianchandotcom
Copy link
Owner

@jbalsas @wincent can you guys auto enforce?

@brianchandotcom
Copy link
Owner

meaning, we need to make sure running prettier again won't break this stuff @jbalsas @wincent

@wincent
Copy link

wincent commented Jun 17, 2019

Possible mitigations:

  • (Temporary) add // prettier-ignore comment before each of these to stop Prettier from touching it.
  • (Hacky) Increase maximum line width setting to reduce likelihood of unwanted line breaks.
  • Adjust Liferay.Language.get regex to match across lines.
  • (Difficult) Make Liferay.Language.get substitution work in terms of AST instead of regular expressions.
  • Patch Prettier to avoid touching these AST nodes (basically a hard-coded version of prettier-ignore).

There might be others but that's what comes to mind for now. @jbalsas and I can discuss and let you know what we come up with.

@wincent
Copy link

wincent commented Jun 18, 2019

@brianchandotcom @inacionery: we're prioritizing https://issues.liferay.com/browse/LPS-97016 to make sure this doesn't happen again.

@jbalsas
Copy link

jbalsas commented Jun 18, 2019

Hey @hhuijser, this PR should've failed ci:sf since it's actually breaking our formatting. Inspecting the log, we see that:

format-source-all:
     [echo] 
     [echo] This target does not format .es.js files. Please run the following command from /opt/dev/projects/github/liferay-portal/modules to do so:
     [echo] 
     [echo] ../gradlew npmRunCheckFormat
     [echo] 				
[stopwatch] [run.batch.test.action: 3:18.158 sec]

I assume we've never really enabled top-level format-source checks for frontend. We've provided top-level scripts that can be run (rather than per-project) so we can enable this.

Could you please reach out to us for more details so we can enable this and make sure we're properly testing this on CI?

Thanks!

@hhuijser
Copy link

Not sure why this was disabled in format-source-all target. Guess because it used to be super slow?

Should be easy to change by making some changes to the ant target in portal-impl/build.xml

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